-
Notifications
You must be signed in to change notification settings - Fork 332
Export inferred parameters related to dependencies #7432
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
base: main
Are you sure you want to change the base?
Export inferred parameters related to dependencies #7432
Conversation
4fd8248
to
4616cb2
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7432 +/- ##
==========================================
- Coverage 59.73% 59.72% -0.01%
==========================================
Files 347 347
Lines 31242 31293 +51
==========================================
+ Hits 18661 18689 +28
- Misses 12581 12604 +23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
b8cde31
to
38abf50
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pinging @samantha-ho for review as well :)
2965bb2
to
1d6e8e5
Compare
1d6e8e5
to
49073aa
Compare
8fb3820
to
0856fba
Compare
bb913ee
to
ea9adc2
Compare
7e15907
to
1f7fce5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the xarray export functionality to include inferred parameters related to dependencies. The changes add support for exporting inferred parameters that have the same shape as setpoints as coordinates, and inferred parameters related to data parameters as data variables.
- Adds new method
to_xarray_dataset_dict
to replace the deprecatedto_xarray_dataarray_dict
- Updates the xarray export logic to include inferred parameters based on their relationships in the dependency graph
- Adds comprehensive test coverage for the new functionality with various parameter relationship scenarios
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/dataset/test_dataset_export.py | Adds type annotations and comprehensive tests for inferred parameter export functionality |
tests/dataset/measurement/test_inferred_parameters_fix.py | Updates existing tests to verify inferred parameters are now exported as coordinates |
src/qcodes/dataset/exporters/export_to_xarray.py | Implements core logic for exporting inferred parameters and adds new to_xarray_dataset_dict method |
src/qcodes/dataset/data_set_protocol.py | Adds protocol definition for new to_xarray_dataset_dict method |
src/qcodes/dataset/data_set_in_memory.py | Implements to_xarray_dataset_dict method and deprecates old method |
src/qcodes/dataset/data_set_cache.py | Adds cache support for new dataset dict export method |
src/qcodes/dataset/data_set.py | Implements to_xarray_dataset_dict method in main DataSet class |
|
||
|
||
@deprecated( | ||
"load_to_xarray_dataarray_dict is deprecated, use load_to_xarray_dataarray_dict instead", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deprecation message incorrectly suggests using the same function being deprecated. It should suggest using "load_to_xarray_dataset_dict" instead.
"load_to_xarray_dataarray_dict is deprecated, use load_to_xarray_dataarray_dict instead", | |
"load_to_xarray_dataarray_dict is deprecated, use load_to_xarray_dataset_dict instead", |
Copilot uses AI. Check for mistakes.
meas.register_custom_parameter("x", paramtype="numeric") | ||
|
||
# Register y as setpoint inferred from basis | ||
meas.register_custom_parameter("y", basis=("x"), paramtype="numeric") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The basis parameter should be passed as a tuple with multiple elements or a single string, not a tuple with one element. Consider using basis="x"
instead of basis=("x")
for single element basis.
meas.register_custom_parameter("y", basis=("x"), paramtype="numeric") | |
meas.register_custom_parameter("y", basis="x", paramtype="numeric") |
Copilot uses AI. Check for mistakes.
Export inferred to and from parameters related to setpoints with the same shape as the setpoints when exporting to xarray and the shape is known. Export inferred related to data parameters as data parameters.
TODO