On Tue, Mar 24, 2015 at 9:24 PM, Josh Smeaton <josh.smea...@gmail.com> wrote:
> Hi,
>
> Firstly (and least importantly) opening a PR makes it a lot easier to review
> code, especially when there are lots of commits. A [WIP] pull request is
> common and useful. If you get a chance, you should open one with this
> change.
>
> I think it's a good idea. So much so that I opened a ticket about a year
> ago: https://code.djangoproject.com/ticket/22394. You'll note some comments
> there about retaining the Year based behaviour as a `BETWEEN X and Y` rather
> than `Extract(YEAR)`. Otherwise, I think the support is rather positive. At
> a high level, your code looks fairly solid and I think would be a useful
> addition.
Thank you for pointing me to this. I have added a PR to that ticket.
Future review and discussion of the changes can continue in the ticket
and PR.
> Another thing I would really like to see is transform based annotations. I'm
> not 100% sure on whether .annotate(F('X__transform')) is supported or not,
> but if it is, we'll get some really nice behaviour from the use of
> transforms.
AFAICT this does not work. Both before and after my change doing:
Article.objects.annotate(month=F('pub_date__month'))
Yields:
---
File "/home/jon/devel/django/tests/lookup/tests.py", line 257, in
test_values_with_month_lookup
values = Article.objects.values('pub_date__month')
File "/home/jon/devel/django/django/db/models/manager.py", line 127,
in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/home/jon/devel/django/django/db/models/query.py", line 703, in values
clone = self._values(*fields)
File "/home/jon/devel/django/django/db/models/query.py", line 698, in _values
query.add_fields(field_names, True)
File "/home/jon/devel/django/django/db/models/sql/query.py", line
1603, in add_fields
name.split(LOOKUP_SEP), opts, alias, allow_many=allow_m2m)
File "/home/jon/devel/django/django/db/models/sql/query.py", line
1377, in setup_joins
names, opts, allow_many, fail_on_missing=True)
File "/home/jon/devel/django/django/db/models/sql/query.py", line
1345, in names_to_path
" not permitted." % (names[pos + 1], name))
FieldError: Cannot resolve keyword u'month' into field. Join on
'pub_date' not permitted.
---
Just to be clear, are you suggesting support for this be a part of my
change? Personally, I see this feature as orthogonal to the transform
refactoring. Seeing as it doesn't work with the built-in lookups nor
transforms there should be little BC concerns.
--
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 django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
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/CADhq2b6Z9P-XH487MKgJ_Sfm_TnMGry-5-WAg6pq33580we5cg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
