Re: Add distinct/group_by support to ndb (issue 7865043)

[arfu...@google.com]

https://codereview.appspot.com/7865043/diff/3001/ndb/model.py
File ndb/model.py (right):

https://codereview.appspot.com/7865043/diff/3001/ndb/model.py#newcode2678
ndb/model.py:2678: def __init__(self, *args, **kwds):
On 2013/03/22 00:05:33, GvR wrote:
> I wouldn't recommend this. While the original code looks truly weird
(and
> probably fails a lint check), the regular style means you can't have a
property
> named 'self'. (Or at least you can't initialize it by calling
> MyModel(self=42).)

ahhh, makes total sense! Thanks (I'll add a comment)

https://codereview.appspot.com/7865043/diff/3001/ndb/model.py#newcode3170
ndb/model.py:3170: if 'group_bys' in kwds:
On 2013/03/22 00:05:33, GvR wrote:
> I really don't like the name 'group_bys'. I notice that some internal
APIs use
> 'group_by' -- I would recommend using that everywhere.
tl;dr; Done

the internal APIs also use 'order' and 'filter'. I was trying to be
consistent (and also leave room for incremental building functions).
However, I whole heartedly agree (though I also wish the singular was
used for order and filter, and the functions had verbs in front of them,
e.g. 'add_order'). Though I can rationalize things this way:
singular: group_by_property
plural: group_by_properties
So if I add an incremental builder function, i can call it
'group_by_property' (the associated function for projection is
'project')

https://codereview.appspot.com/7865043/diff/3001/ndb/query.py
File ndb/query.py (right):

https://codereview.appspot.com/7865043/diff/3001/ndb/query.py#newcode780
ndb/query.py:780: group_bys: Optional list or tuple of properties to
group by.
On 2013/03/22 00:05:33, GvR wrote:
> group_by.

Done.

https://codereview.appspot.com/7865043/diff/3001/ndb/query.py#newcode1384
ndb/query.py:1384: # Populate projection if it hasn't been overriden
On 2013/03/22 00:05:33, GvR wrote:
> Period.

Done.

https://codereview.appspot.com/7865043/

[gvanr...@gmail.com]

LGTM


https://codereview.appspot.com/7865043/diff/17001/ndb/query.py
File ndb/query.py (right):

https://codereview.appspot.com/7865043/diff/17001/ndb/query.py#newcode1384
ndb/query.py:1384: # Populate projection if it hasn't been overriden.
Two dees as wel...

https://codereview.appspot.com/7865043/