Fix 7717, Improve handling of data variables with inferred parameters + tests by jenshnielsen · Pull Request #7725 · microsoft/Qcodes

jenshnielsen · 2025-12-15T10:32:59Z

Add failing test based on Dataset converts to ungridded data if two ParameterWithSetpoints share inference and setpoints #7717
Closes Dataset converts to ungridded data if two ParameterWithSetpoints share inference and setpoints #7717

jenshnielsen · 2026-02-02T10:18:50Z

@astafan8 converted to draft until it actually fixes the bug

codecov · 2026-03-18T09:06:01Z

Codecov Report

❌ Patch coverage is 79.41176% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.61%. Comparing base (8bde5fa) to head (6923a00).
⚠️ Report is 15 commits behind head on main.

Files with missing lines	Patch %	Lines
src/qcodes/dataset/exporters/export_to_xarray.py	78.12%	7 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7725   +/-   ##
=======================================
  Coverage   70.60%   70.61%           
=======================================
  Files         333      333           
  Lines       32502    32533   +31     
=======================================
+ Hits        22947    22972   +25     
- Misses       9555     9561    +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

astafan8

great to see this fixed! And "add a news fragment"?

Copilot

Pull request overview

This PR addresses #7717, where a dataset export to xarray becomes ungridded when two ParameterWithSetpoints share the same setpoints and one parameter is inferred/controlled by the other. The intent is to ensure the export stays gridded and includes the inferred parameter data alongside the measured top-level parameter.

Changes:

Adds a regression test reproducing the ParameterWithSetpoints.has_control_of + shared setpoints scenario.
Updates InterDependencies_.top_level_parameters to exclude parameters that are inferred-from others from being considered dependency top-level parameters.
Extends the xarray export (pandas-index path) to add inferred parameters as xarray data variables.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
`tests/dataset/test_parameter_with_setpoints_has_control.py`	Regression test ensuring gridded xarray export includes inferred `p1` as a data var while keeping only `p2` top-level.
`src/qcodes/dataset/exporters/export_to_xarray.py`	Adds `_add_inferred_data_vars` to include inferred parameters in xarray output when exporting via pandas index.
`src/qcodes/dataset/descriptions/dependencies.py`	Adjusts top-level parameter detection to prevent inferred parameters from being treated as independent top-level parameters.

You can also share your feedback on Copilot code review. Take the survey.

Copilot

Pull request overview

Fixes QCoDeS dataset export behavior related to issue #7717, where ParameterWithSetpoints parameters involved in has_control_of/inference relationships could cause to_xarray_dataset() to export ravelled (ungridded) data instead of a properly gridded dataset.

Changes:

Add regression tests covering 1D and 2D ParameterWithSetpoints + has_control_of inference export behavior, plus a warning-path test.
Adjust InterDependencies_.top_level_parameters to avoid treating inferred parameters as independent top-level parameters.
Extend xarray export to include inferred parameters as data variables when exporting via the pandas-index path, warning when inferred data sizes don’t match the parent parameter.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
`tests/dataset/test_parameter_with_setpoints_has_control.py`	Adds regression tests reproducing #7717 and validating inferred parameters appear correctly in xarray export.
`src/qcodes/dataset/exporters/export_to_xarray.py`	Adds `_add_inferred_data_vars` and integrates it into the pandas-index export paths to include inferred parameters.
`src/qcodes/dataset/descriptions/dependencies.py`	Updates `top_level_parameters` to exclude parameters inferred from others from dependency top-level candidates.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Instead of assuming a single parent by taking inferred_from_params[0], iterate over all inferred-from parents and use the first one whose flattened data size matches. Log a warning listing all available parents when no match is found. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Check inferred parameter data size against the xr_dataset dimensions directly instead of iterating over individual parent sizes. The previous logic could match a parent whose size differed from the dataset grid, causing a ValueError on reshape. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Fix newsfragment wording to match actual behavior (checks dataset dimensions, not parent parameter size) - Extract _make_controlled_setpoints helper to reduce MySp class duplication across tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jenshnielsen requested a review from a team as a code owner December 15, 2025 10:33

astafan8 reviewed Feb 2, 2026

View reviewed changes

Comment thread tests/dataset/test_parameter_with_setpoints_has_control.py Outdated

astafan8 approved these changes Feb 2, 2026

View reviewed changes

jenshnielsen marked this pull request as draft February 2, 2026 10:18

jenshnielsen force-pushed the fix_7717 branch from 4b1792e to 175b160 Compare March 18, 2026 08:22

jenshnielsen marked this pull request as ready for review March 18, 2026 09:06

astafan8 approved these changes Mar 18, 2026

View reviewed changes

Comment thread tests/dataset/test_parameter_with_setpoints_has_control.py

jenshnielsen requested a review from Copilot March 18, 2026 12:11

Copilot started reviewing on behalf of jenshnielsen March 18, 2026 12:12 View session

jenshnielsen commented Mar 18, 2026

View reviewed changes

Comment thread src/qcodes/dataset/exporters/export_to_xarray.py

Copilot AI reviewed Mar 18, 2026

View reviewed changes

Comment thread src/qcodes/dataset/exporters/export_to_xarray.py

jenshnielsen force-pushed the fix_7717 branch from 3c254d8 to 50e6dab Compare March 23, 2026 09:58

jenshnielsen requested review from astafan8 and Copilot March 23, 2026 10:01

Copilot started reviewing on behalf of jenshnielsen March 23, 2026 10:01 View session

astafan8 approved these changes Mar 23, 2026

View reviewed changes

Copilot AI reviewed Mar 23, 2026

View reviewed changes

Comment thread src/qcodes/dataset/exporters/export_to_xarray.py Outdated

Comment thread src/qcodes/dataset/exporters/export_to_xarray.py

Comment thread tests/dataset/test_parameter_with_setpoints_has_control.py

jenshnielsen requested a review from Copilot April 14, 2026 08:55

jenshnielsen force-pushed the fix_7717 branch from 12ab522 to 17a6971 Compare April 14, 2026 09:01

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Comment thread docs/changes/newsfragments/7725.improved Outdated

Comment thread tests/dataset/test_parameter_with_setpoints_has_control.py Outdated

Comment thread src/qcodes/dataset/exporters/export_to_xarray.py

jenshnielsen changed the title ~~Fix 7717~~ Fix 7717, Improve handling of data variables with inferred parameters + tests Apr 14, 2026

Copilot started reviewing on behalf of jenshnielsen April 14, 2026 09:16 View session

jenshnielsen added 6 commits May 1, 2026 13:14

Add failing test based on the bug report in 7717

2506a33

Fix 7717

24ba7e7

Remove outdated comments

83cd3a5

Improve test for 7717

e096556

Add infeered data to exported dataset

719e722

Only add data if shape matches

288469e

jenshnielsen and others added 8 commits May 1, 2026 13:14

Add additional test for 7717

83afba4

Add warning on invalid shape

15cac32

Add changelog for 7725

3a531d5

Fix NPY002: use np.random.default_rng() in inferred params tests

d96553a

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Fix NPY002: use np.random.default_rng() in setpoints control tests

be084ec

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jenshnielsen force-pushed the fix_7717 branch from 01b7f12 to 6923a00 Compare May 1, 2026 11:14

jenshnielsen enabled auto-merge May 1, 2026 11:30

jenshnielsen added this pull request to the merge queue May 1, 2026

Merged via the queue into microsoft:main with commit ef29d25 May 1, 2026
22 of 23 checks passed

jenshnielsen deleted the fix_7717 branch May 1, 2026 12:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix 7717, Improve handling of data variables with inferred parameters + tests#7725

Fix 7717, Improve handling of data variables with inferred parameters + tests#7725
jenshnielsen merged 14 commits intomicrosoft:mainfrom
jenshnielsen:fix_7717

jenshnielsen commented Dec 15, 2025 •

edited

Loading

Uh oh!

Uh oh!

jenshnielsen commented Feb 2, 2026

Uh oh!

codecov Bot commented Mar 18, 2026 •

edited

Loading

Uh oh!

astafan8 left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jenshnielsen commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jenshnielsen commented Feb 2, 2026

Uh oh!

codecov Bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

astafan8 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jenshnielsen commented Dec 15, 2025 •

edited

Loading

codecov Bot commented Mar 18, 2026 •

edited

Loading