Hi Josh,

Thanks so much for your input, both here and on the ticket I referenced.

You were correct, MonthTransform did work. Essentially, this piece of code 
worked in django 1.9:

    MyModel.objects.filter(date1__month=MonthTransform('date2'))


However, the way I see it here, the right hand side lookup interface is all 
broken up and scattered across multiple modules and classes, many of which 
are undocumented.

I am of the opinion that there should be a unified right hand side lookup 
interface, whether it resided in lookups or expressions, but it should be 
possible to just use one class for all types of right hand side 
expressions/lookups.
The left hand side lookups work just fine and are able to figure out 
everything about the type of field and the type of lookup and translate 
that into sql, so why shouldn't the right hand side ones be able to do the 
same? 

The documentation does say that the F() object represents the value of a 
model field, but is there a reason why it couldn't be the universal right 
hand side lookup operator, inclusive of 'subfields' (in a manner of 
speaking) such as year, month and day on DateFields (essentially, a subset 
of lookups allowable on the left hand side, such that gt would not be 
allowed, for instance), so that the following would work:

    MyModel.objects.filter(date1__month=F('date2__month'))


Additionally, is there a reason why Combinable/F couldn't figure out its 
own output_field type (at least in some cases? e.g (DateField - DateField) 
--> DurationField) , as opposed to having to use an ExpressionWrapper, 
which makes the interface slightly more complicated?

What do you think, Josh?


Best regards,
     David Filipovic

On Thursday, October 15, 2015 at 11:21:40 PM UTC-4, Josh Smeaton wrote:
>
> Hi David,
>
> There are a few undocumented expressions in the expressions module because 
> they aren't intended to be used by the public. The Date and DateTime 
> expressions you reference were actually moved from elsewhere (if memory 
> serves) during the expressions refactor. They were previously very thin 
> data wrappers used to represent dates in columns. Some of the functionality 
> was pulled out of query.py and moved to inside the expression. They're the 
> result of a refactor, which is why they weren't given their own tests I 
> assume. However, they should probably have been tested before.
>
> If you can make a case for making these types public then please do so. By 
> making them public we'd document them and write some tests to ensure they 
> do what they say they do. From a cursory inspection these two particular 
> types look like helper classes for F() expressions, so I'm not exactly sure 
> of their public utility. So if you'd like to see them made public (or 
> partially public - as building blocks for other expressions), then please 
> show how you'd use them and we can consider doing so. This goes for any 
> other type in the expressions/lookups namespaces by the way. Unless utility 
> is immediately apparent we'd default to private, but always open to making 
> something public.
>
> Regarding coding style, there's 
> https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/.
>  
> More generally there's 
> https://docs.djangoproject.com/en/dev/internals/contributing/ and 
> https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/. 
> Other than that, bikeshedding goes a long way towards naming (code review 
> focusing on trivialities).
>
> It'd be really great to have some more contributors helping with 
> expressions and the ORM in general so, welcome! The mailing list is good 
> for getting some long form answers, but feel free to join us on IRC at 
> #django-dev to bounce some ideas off or get some more clarification.
>
> Regards,
>
> On Friday, 16 October 2015 05:43:48 UTC+11, David Filipovic wrote:
>>
>> Hi all,
>>
>> I have recently stumbled upon a very useful feature missing from 
>> `django.db.models.expressions`.
>>
>> I came up with a patch and followed the guidelines to create my ticket: 
>> https://code.djangoproject.com/ticket/25556
>> and pull request: https://github.com/django/django/pull/5431
>>
>> One thing I'm confused about is the fact that I would be adding new 
>> classes/functionality to the system (so I would really want to write tests 
>> for it), however similar classes in the same module seem not to be covered 
>> by tests at all. For instance, the only test for 
>> `django.db.models.expressions.Date` I could find was the one testing its 
>> `__repr__` method, which doesn't seem all that useful. 
>>
>> One thing that's noticeable is that a lot of the more complex query 
>> expressions present there (like Date and DateTime) are not really covered 
>> by the documentation either (even though Date is used in `QuerySet.dates()` 
>> which is well documented). Are these features considered undocumented, and 
>> is there a plan to make them officially supported (because they're really 
>> useful)?
>>
>>
>> One other thing I wanted to inquire about is what is the best naming 
>> practice for django when adding new classes/methods/functions? I couldn't 
>> find anything on this.
>>
>> Thank you so much. I hope I can contribute and help make django better!
>>
>> Best regards,
>>      David Filipovic
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/a9beaf4d-2bff-48c1-bbd3-8222bb38019f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to