feat: Allow iterable of ids in retrieve methods for datapoints#2583
feat: Allow iterable of ids in retrieve methods for datapoints#2583MortGron wants to merge 4 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request generalizes the input types for datapoint retrieval methods, replacing Sequence and SequenceNotStr with Iterable across the API and utility modules. Feedback was provided to explicitly disallow dict inputs in the identifier parsing logic, as dictionaries are iterables that could lead to confusing error messages if not handled early.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the datapoints API to support Iterable types for identifiers, allowing the use of collections like sets or dictionary keys instead of being restricted to sequences. The changes include updates to type hints, docstrings, and internal parsing logic across the async and sync APIs. Feedback indicates that _FullDatapointsQuery.is_single_identifier and the retrieve_latest method were not updated to handle the new Iterable support, which may result in incorrect return types or ignored settings for certain collection types.
| id (None | int | DatapointsQuery | Iterable[int | DatapointsQuery]): Id, DatapointsQuery or (mixed) iterable of these. If a dict is passed, its keys will be used as ids. See examples below. | ||
| external_id (None | str | DatapointsQuery | Iterable[str | DatapointsQuery]): External id, DatapointsQuery or (mixed) iterable of these. If a dict is passed, its keys will be used as external_ids. See examples below. | ||
| instance_id (None | NodeId | DatapointsQuery | Iterable[NodeId | DatapointsQuery]): Instance id, DatapointsQuery or (mixed) iterable of these. If a dict is passed, its keys will be used as instance_ids. |
There was a problem hiding this comment.
The overloads for retrieve_latest (starting at line 1642) and its internal helper RetrieveLatestDpsFetcher._parse_user_input (line 2552) were not updated to support Iterable and still rely on Sequence and MutableSequence. This results in an inconsistent API where set or tuple of LatestDatapointQuery objects will not be correctly processed, and their individual settings (like before or target_unit) will be ignored.
There was a problem hiding this comment.
I think what Gemini asks for here is consistency across the retrieve datapoints methods. I personally am ok with incrementally chaning the methods to accept Iterable of IDs, that is, skipping retrieve_latest for now. What do you think @cognitedata/python-sdk-maintainers ?
1 participant
Description
It is common that the IDs the users fead to the retreive methods are in a
setordictor other non-sequence type.This PRs allow the IDs be
Iterablein the datapoints retrieve methods.Checklist:
If a new method has been added it should be referenced in cognite.rst in order to generate docs based on its docstring.