Implements group_by, projection and offset on datastore Query.

[dhermes]

In addition, __eq__ was implemented on datastore.key.Key to allow for easy comparison.

See #281 for some context. As mentioned there, I looked and didn't see any equivalent bugs for these features.

[tseaver]

How is this PR different from #281?

[dhermes]

From #281:

As new Query features are outside the scope of functional / regression testing, I am going
to peel them off into a separate PR and then rebase this one onto it after that is reviewed.
However, please feel free to dig into this review.

This PR has only the new features and none of the regression tests.

[dhermes]

Rebased after the Travis fix.

@tseaver PTAL

[coveralls]

Coverage remained the same when pulling 8df6f41 on dhermes:implement-groupby-projection-offset into 9884f77 on GoogleCloudPlatform:master.

[tseaver]

Implementation LGTM, although I don't know these bits of the API at all. Does 'offset()' conflict with the cursor stuff?

[dhermes]

No, they are complimentary. We can get final sign-off from one of the datastore folks if you like?

[coveralls]

Coverage remained the same when pulling 3d4ce5d on dhermes:implement-groupby-projection-offset into 2da8654 on GoogleCloudPlatform:master.

[dhermes]

@tseaver while fixing some failures in #281 I realized #274 made key equality a bit of a different animal.

I added b61ba2e to address some corner cases.

You'll notice that this is a bit unpleasant. ISTM we should work harder to make the underlying representation of a Key simpler. /cc @pcostell

[coveralls]

Coverage remained the same when pulling b61ba2e on dhermes:implement-groupby-projection-offset into 2da8654 on GoogleCloudPlatform:master.

[tseaver]

I still don't see why two keys, each with 'dataset_id == None' but having the same path, should be unequal (they are both in the "default" dataset for the application's connection).

Likewise for when both keys have 'namespace == None' but the same path.

[dhermes]

They would both be equal. See test_key___eq__.

The failures happen if (dataset as an example)

        if not (self._dataset_id == other._dataset_id or
                self._dataset_id is None or other._dataset_id is None):

which is equivalent to

        if (self._dataset_id != other._dataset_id and
            self._dataset_id is not None and other._dataset_id is not None):

In other words, failure only occurs if the values don't match AND both values are non-null.

[tseaver]

I don't understand why it isn't just:

if self._dataset_id != other._dataset_id:

If they are both None, they will be equal; if either is not None but the other is, they won't be equal. None compares for equality fine with arbitrary objects.

[dhermes]

• Dataset: Because of Fix #121: don't copy project ID to the Key protobuf's dataset_id. #276 (sorry was referring to Revert "Fix #121: Drop munging of 's~' prefix onto keys." #274 before) the values may be unset.
• Namespace: The same happens with values of None (local) vs u'' (from retrieved protobuf).

To give a simplified idea of what is happening in test_post_with_generated_id, test_post_with_id and test_post_with_name:

>>> import regression.regression_utils
>>> dataset = regression.regression_utils.get_dataset()
>>> entity = dataset.entity(kind='SomeKind')
>>> entity['key'] = 'value'
>>> key = entity.key().name('some_name')  # Same for partial key or key id.
>>> entity = entity.key(key)
>>> entity.save()
<Entity[{'kind': 'SomeKind', 'name': 'some_name'}] {'key': 'value'}>
>>> retrieved_entity = dataset.get_entity(entity.key())

then we'll have

>>> entity.key().path()
[{'kind': 'SomeKind', 'name': 'some_name'}]
>>> retrieved_entity.key().path()
[{'kind': u'SomeKind', 'name': u'some_name'}]
>>> 
>>> print entity.key()._dataset_id
None
>>> retrieved_entity.key()._dataset_id
u's~redacted-dataset-id'
>>>
>>> print entity.key()._namespace
None
>>> retrieved_entity.key()._namespace
u''

[coveralls]

Coverage remained the same when pulling c27a8d5 on dhermes:implement-groupby-projection-offset into 09859db on GoogleCloudPlatform:master.

[dhermes]

Rebased after #289.

@tseaver does the explanation above make sense for allowing null values in __eq__? Do you think we should do some more work to make these values actually agree?

• For Datasets: I suppose much of this is what was removed in Fix #121: don't copy project ID to the Key protobuf's dataset_id. #276
• For namespaces we can either switch our default from None to '' or we can set _namespace to None when it is '' on the protobuf

[coveralls]

Coverage remained the same when pulling 330ffc5 on dhermes:implement-groupby-projection-offset into 8a21978 on GoogleCloudPlatform:master.

[tseaver]

Yes, I see what you are trying to accomplish now. WRT the key's _datset_id, I'm not positive we want None to compare equal to one with an ID set by the back-end: it is sort of like comparing a naive datetime to a zoned one: they will be the same now, but later (if multi-dataset access comes into play) they might not be.

WRT namespace: we could switch the default value from None to '', and then just compare them naturally.

[coveralls]

Coverage remained the same when pulling 95528d2 on dhermes:implement-groupby-projection-offset into b6d3e74 on GoogleCloudPlatform:master.

[dhermes]

@tseaver it turns out we weren't handling namespaces quite perfectly. See #292.

As for dataset ID, the comment I added in Entity.save() to #292 may be the best strategy (i.e. if the key starts implicit, it should stay implicit).

To fully support this, we'd (at least) need to change Dataset.get_entities since it just creates each entity via helpers.entity_from_protobuf without any regard to the original values in keys().

A simple solution to this could be changing

        for entity_pb in entity_pbs:
            entities.append(helpers.entity_from_protobuf(
                entity_pb, dataset=self))

to

        for entity_pb, curr_key in zip(entity_pbs, keys):
            curr_entity = helpers.entity_from_protobuf(
                entity_pb, dataset=self)
            # If original key was implicit, maintain this on the
            # returned entity.
            if curr_key._dataset_id is None:
                curr_entity.key()._dataset_id = None
            else:
                if curr_entity.key() != curr_key:
                    raise ValueError('Key returned disagrees with key used '
                                     'in query.')
            entities.append(curr_entity)