Skip to content

Commit 01b7f12

Browse files
jenshnielsenCopilot
andcommitted
Address PR review comments
- 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>
1 parent 8c5df74 commit 01b7f12

2 files changed

Lines changed: 27 additions & 24 deletions

File tree

docs/changes/newsfragments/7725.improved

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ xarray. Controlled parameters are no longer treated as independent top-level
33
parameters, preventing duplicate data rows. Additionally, inferred parameters
44
are now included as data variables in the xarray dataset when exporting via the
55
pandas-based path, and a warning is logged when the inferred parameter data size
6-
does not match its parent parameter.
6+
does not match the expected xarray dataset dimensions.

tests/dataset/test_parameter_with_setpoints_has_control.py

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
from qcodes.dataset import Measurement
99
from qcodes.dataset.exporters.export_to_xarray import _add_inferred_data_vars
10-
from qcodes.parameters import ManualParameter, ParameterWithSetpoints
10+
from qcodes.parameters import ManualParameter, Parameter, ParameterWithSetpoints
1111
from qcodes.validators import Arrays
1212

1313
if TYPE_CHECKING:
@@ -16,22 +16,35 @@
1616
from qcodes.dataset.experiment_container import Experiment
1717

1818

19-
def test_parameter_with_setpoints_has_control(experiment: "Experiment"):
20-
class MySp(ParameterWithSetpoints):
21-
def unpack_self(self, value):
19+
def _make_controlled_setpoints(
20+
name: str,
21+
controlled: Parameter,
22+
**kwargs: object,
23+
) -> ParameterWithSetpoints:
24+
"""Create a ParameterWithSetpoints that infers ``controlled`` via unpack_self."""
25+
26+
class _ControlledSetpoints(ParameterWithSetpoints):
27+
def unpack_self(self, value): # type: ignore[override]
2228
res = super().unpack_self(value)
23-
res.append((p1, p1()))
29+
res.append((controlled, controlled()))
2430
return res
2531

32+
p = _ControlledSetpoints(name, **kwargs) # type: ignore[arg-type]
33+
p.has_control_of.add(controlled)
34+
return p
35+
36+
37+
def test_parameter_with_setpoints_has_control(experiment: "Experiment"):
2638
mp_data = np.arange(10)
2739
p1_data = np.linspace(-1, 1, 10)
2840

2941
mp = ManualParameter("mp", vals=Arrays(shape=(10,)), initial_value=mp_data)
3042
p1 = ParameterWithSetpoints(
3143
"p1", vals=Arrays(shape=(10,)), setpoints=(mp,), set_cmd=None
3244
)
33-
p2 = MySp("p2", vals=Arrays(shape=(10,)), setpoints=(mp,), set_cmd=None)
34-
p2.has_control_of.add(p1)
45+
p2 = _make_controlled_setpoints(
46+
"p2", p1, vals=Arrays(shape=(10,)), setpoints=(mp,), set_cmd=None
47+
)
3548

3649
p1(p1_data)
3750
p2_data = np.random.default_rng().standard_normal(10)
@@ -81,12 +94,6 @@ def test_parameter_with_setpoints_has_control_2d(experiment: "Experiment"):
8194
"""Test that an inferred parameter with the same size as its parent
8295
but different from the full dimension product is correctly included."""
8396

84-
class MySp(ParameterWithSetpoints):
85-
def unpack_self(self, value):
86-
res = super().unpack_self(value)
87-
res.append((p1, p1()))
88-
return res
89-
9097
n_x = 3
9198
n_y = 4
9299
mp_x_data = np.arange(n_x, dtype=float)
@@ -98,8 +105,9 @@ def unpack_self(self, value):
98105
p1 = ParameterWithSetpoints(
99106
"p1", vals=Arrays(shape=(n_y,)), setpoints=(mp_y,), set_cmd=None
100107
)
101-
p2 = MySp("p2", vals=Arrays(shape=(n_y,)), setpoints=(mp_y,), set_cmd=None)
102-
p2.has_control_of.add(p1)
108+
p2 = _make_controlled_setpoints(
109+
"p2", p1, vals=Arrays(shape=(n_y,)), setpoints=(mp_y,), set_cmd=None
110+
)
103111

104112
meas = Measurement()
105113
meas.register_parameter(p2, setpoints=(mp_x,))
@@ -145,20 +153,15 @@ def test_parameter_with_setpoints_has_control_size_mismatch_warns(
145153
"""Test that a warning is emitted when the inferred parameter has a
146154
different data size than its parent parameter."""
147155

148-
class MySp(ParameterWithSetpoints):
149-
def unpack_self(self, value):
150-
res = super().unpack_self(value)
151-
res.append((p1, p1()))
152-
return res
153-
154156
mp_data = np.arange(10)
155157

156158
mp = ManualParameter("mp", vals=Arrays(shape=(10,)), initial_value=mp_data)
157159
p1 = ParameterWithSetpoints(
158160
"p1", vals=Arrays(shape=(10,)), setpoints=(mp,), set_cmd=None
159161
)
160-
p2 = MySp("p2", vals=Arrays(shape=(10,)), setpoints=(mp,), set_cmd=None)
161-
p2.has_control_of.add(p1)
162+
p2 = _make_controlled_setpoints(
163+
"p2", p1, vals=Arrays(shape=(10,)), setpoints=(mp,), set_cmd=None
164+
)
162165

163166
p1(np.linspace(-1, 1, 10))
164167
p2(np.random.default_rng().standard_normal(10))

0 commit comments

Comments
 (0)