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

Added chain to and direction argument to expression result sets #1528

Merged
merged 8 commits into from
Nov 30, 2023

Conversation

doctrino
Copy link
Contributor

@doctrino doctrino commented Nov 28, 2023

Description

New parameter:
image

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.

@doctrino doctrino changed the title t: Added chain to argument to expressoin result sets Added chain to argument to expressoin result sets Nov 28, 2023
Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Merging #1528 (fea46b9) into master (b0bdb05) will decrease coverage by 0.10%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1528      +/-   ##
==========================================
- Coverage   91.83%   91.73%   -0.10%     
==========================================
  Files         120      120              
  Lines       15584    15591       +7     
==========================================
- Hits        14311    14302       -9     
- Misses       1273     1289      +16     
Files Coverage Δ
cognite/client/_api/workflows.py 81.35% <ø> (ø)
cognite/client/_version.py 100.00% <100.00%> (ø)
cognite/client/data_classes/data_modeling/query.py 90.47% <100.00%> (+0.36%) ⬆️

... and 4 files with indirect coverage changes

@doctrino doctrino force-pushed the add-chain-to-parameter branch from adc297d to 26a8415 Compare November 28, 2023 19:03
@doctrino doctrino changed the title Added chain to argument to expressoin result sets Added chain to argument to expression result sets Nov 28, 2023
@doctrino doctrino marked this pull request as ready for review November 28, 2023 19:05
@doctrino doctrino requested review from a team as code owners November 28, 2023 19:05
@doctrino doctrino changed the title Added chain to argument to expression result sets Added chain to and direction argument to expression result sets Nov 28, 2023
tests/utils.py Outdated
@@ -308,8 +308,12 @@ def create_instance(self, resource_cls: type[T_Object]) -> T_Object:
# DataPointSubscriptionCreate requires either timeseries_ids or filter
keyword_arguments.pop("filter", None)
if resource_cls is Query:
key_count = min(len(keyword_arguments["select"]), len(keyword_arguments["with_"]))
Copy link
Collaborator

Choose a reason for hiding this comment

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

There should never be fewer result expressions than selects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is from the FakeCogniteResourceGenerator this is just to ensure that the with_ and select arguments ends up with the same number of entries (they are both randomly generated based on the init signature, and since they are dict they are of random lengths). I have simplified the code to attempt to make it clearer.

@doctrino doctrino force-pushed the add-chain-to-parameter branch from 43a6909 to d4c09f2 Compare November 30, 2023 14:47
Copy link
Collaborator

@erlendvollset erlendvollset left a comment

Choose a reason for hiding this comment

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

LGTM!

@doctrino doctrino merged commit e2624e6 into master Nov 30, 2023
7 checks passed
@doctrino doctrino deleted the add-chain-to-parameter branch November 30, 2023 15:01
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