cognite-sdk-python
cognite-sdk-python copied to clipboard
Improve Cognite base classes
Description
New faster implementation for _cognite_client
private attr:
- Removes the custom
__new__
implementation for a 2X object instantiation speed up. - With
__new__
removed, subclasses ofCogniteResource
no longer have an implicit__init__
that just swallows allargs
and allkwargs
. - Removes custom
__getattribute__
implementation (extra hook before regular attribute lookup). This change adds a speed up to all attribute accesses. - Adds
_cognite_client
as a simple property (see class_WithClientMixin
) with a setter method that guarantees a valid value (Optional[CogniteClient]
) and a getter that raisesCogniteMissingClientError
in all cases except where the attribute is an instance ofCogniteClient
.
New faster _load
implementation
- Removes the old
if hasattr -> setattr
implementation that calledto_snake_case
O(N*M)
times (N
dicts,M
elements per dict). - Changed to using the newly added
fast_dict_load
function, which usesget_accepted_params
that caches the accepted parameters for each class it receives, returning a dict that mapscamelCase
tosnake_case
for very efficient API response translation into our data classes.
New hierarchy to avoid code duplication
- Introduces a
CogniteBase
class with__str__
,__repr__
,__eq__
anddump
methods. - Introduces a
CogniteBaseList
class which...- Adds all missing dunder- and list methods necessary for us to override from
UserList
. - This means we no longer leak memory because of the internal dict lookup on
id
andexternal_id
that keeps a reference to elements after deletion from the list. - This also adds duplicate checks that raise on non-preserving actions.
- Adds all missing dunder- and list methods necessary for us to override from
- Adds the missing
CogniteResponseList
(note not Resource)
Other changes / optimizations:
-
_load
methods now forcecognite_client
to be passed as these are "only" used by the SDK. This allows better visibility into all the different classes that do not use it (and could be moved to a different parent class without the client in the future). - With these changes,
mypy
now better understands the code base. This has resulted in 100+ issues that have been changed or fixed. - The error message on
CogniteMissingClientError
has been improved. - New tests / check for the various data classes: Verification of base subclasses has been moved to tests (
TestVerifyAllCogniteSubclasses
). -
CogniteResponse
no longer acceptscognite_client
. -
CogniteFilter
and subclasses had many unused_load
functions that have been removed (these are never instantiated by the SDK, as opposed to most other resource classes). - Removes
EXCLUDE_VALUE
in favor of a simpleNone
-check (was list search with one element). YAGNI 😄
Codecov Report
Merging #1151 (f4807c6) into master (05e2b3f) will increase coverage by
0.25%
. The diff coverage is82.13%
.
@@ Coverage Diff @@
## master #1151 +/- ##
==========================================
+ Coverage 90.63% 90.89% +0.25%
==========================================
Files 82 82
Lines 9899 9906 +7
==========================================
+ Hits 8972 9004 +32
+ Misses 927 902 -25
Impacted Files | Coverage Δ | |
---|---|---|
cognite/client/_api/datapoints.py | 96.30% <ø> (ø) |
|
cognite/client/_api/sequences.py | 95.75% <ø> (ø) |
|
cognite/client/_api/three_d.py | 97.61% <ø> (ø) |
|
cognite/client/data_classes/annotations.py | 96.38% <ø> (-0.05%) |
:arrow_down: |
cognite/client/data_classes/data_sets.py | 91.04% <ø> (+7.92%) |
:arrow_up: |
cognite/client/data_classes/events.py | 93.00% <ø> (+10.24%) |
:arrow_up: |
cognite/client/data_classes/raw.py | 88.46% <66.66%> (-9.04%) |
:arrow_down: |
cognite/client/exceptions.py | 88.88% <66.66%> (+0.22%) |
:arrow_up: |
...ognite/client/data_classes/transformations/jobs.py | 89.58% <71.42%> (-1.73%) |
:arrow_down: |
cognite/client/data_classes/_base.py | 86.09% <77.56%> (-8.98%) |
:arrow_down: |
... and 24 more |
Interested in feedback on how to make the class hierarchy better!
- Type hints still feel lacking in many places, should we add
typing_extensions ^4
to getSelf
for instance?