Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In filter with node ids #1991

Merged
merged 6 commits into from
Nov 2, 2024
Merged

In filter with node ids #1991

merged 6 commits into from
Nov 2, 2024

Conversation

MortGron
Copy link
Contributor

@MortGron MortGron commented Oct 22, 2024

Description

If you pass a list of NodeIds as values to the In filter and use this to filter instances in a data model, you get the following error:
Invalid value for direct relation property 'property'. The value must be a json object with space and externalId fields. | code: 400
The same happens if you try to use the dump method on the NodeIds. You have to relalize that include_instance_type has to be set to False, which is not obvious by reading the error message.

Checklist:

  • Tests added/updated.
  • Documentation updated. Documentation is generated from docstrings - these must be updated according to your change.
    If a new method has been added it should be referenced in cognite.rst in order to generate docs based on its docstring.
  • Changelog updated in CHANGELOG.md.
  • Version bumped. If triggering a new release is desired, bump the version number in _version.py and pyproject.toml per semantic versioning.

Copy link

codecov bot commented Oct 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.26%. Comparing base (0941557) to head (44b7ccf).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1991      +/-   ##
==========================================
- Coverage   90.27%   90.26%   -0.01%     
==========================================
  Files         139      139              
  Lines       21882    21885       +3     
==========================================
+ Hits        19753    19754       +1     
- Misses       2129     2131       +2     
Files with missing lines Coverage Δ
cognite/client/_version.py 100.00% <100.00%> (ø)
cognite/client/data_classes/filters.py 95.67% <100.00%> (+0.04%) ⬆️

... and 1 file with indirect coverage changes

@MortGron MortGron marked this pull request as ready for review October 22, 2024 10:17
@MortGron MortGron requested review from a team as code owners October 22, 2024 10:17
Copy link
Contributor

@haakonvt haakonvt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Another great addition! 🔥 A few comments left for you before you merge:

cognite/client/data_classes/filters.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@MortGron MortGron merged commit f8aa382 into master Nov 2, 2024
16 checks passed
@MortGron MortGron deleted the in-filter-with-node-ids branch November 2, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants