Parameter.__getitem__ behavior

[thangleiter]

Parameters can be indexed to return SweepFixedValues. This is a handy shortcut if you know it exists and use it correctly. However, it can lead to extremely unpredictable behavior when parameters are used in the wrong context, for example if a Parameter is passed to a function that expects an Iterable.

The function might try to convert the Iterable to a list and calls list(param), which runs forever if the parameter's validator passes all positive integers. In my opinion, this is very unhandy behavior and not worth the syntactic sugar of writing param[:10] instead of having a method that achieves the same thing without the unintended consequences of making Parameter iterable.

An example, where the unassuming user passes a Parameter instead of a sequence of parameters as setpoints to Measurement.register_parameter:

from qcodes.parameters import ManualParameter, ParameterWithSetpoints
from qcodes.validators import Arrays
from qcodes.dataset import Measurement

getme = ManualParameter('foo')
setme = ManualParameter('bar')

meas = Measurement()
meas.register_parameter(setme)
meas.register_parameter(getme, setpoints=setme)
# should error, since setpoints should be Sequence[Union[str, "ParameterBase"]]
# instead, raises obscure error
AttributeError: 'SweepFixedValues' object has no attribute 'register_name'

For ParameterWithSetpoints, register_parameter ends up trying to convert the argument to a list:

getme_setpoints = ParameterWithSetpoints('baz', setpoints=(setme,), vals=Arrays(shape=(10,)))

meas = Measurement()
meas.register_parameter(setme)
meas.register_parameter(getme_setpoints, setpoints=setme)
# should error, instead runs forever

In this case, it would even be cumbersome to check in _register_parameter_with_setpoints() if setpoints is the correct type, i.e., , because iter(setpoints) works even if setpoints is a Parameter! (*Well, Parameter is not an instance of Sequence, so not too cumbersome.)

Also runs forever:

for _ in setme:
    pass

My proposal is therefore to deprecate Parameter.__getitem__ and instead introduce a method like Parameter.sweep(keys: int | slice) or even mirror xarray and call it Parameter.iloc or something.

[jenshnielsen]

It should indeed be deprecated see https://github.com/microsoft/Qcodes/pull/5036

__getitem__ and parameter.sweep (which already exists) are both remaining leftovers of qcodes_loop in qcodes that I have not yet found a clean way to get rid of without breaking that package.

We are not planning to introduce any sweep functionality into the parameter to use with qcodes.dataset but parameters can be sweep by passing them to LinSweep and friends. This choice is made since

• The parameter class is already too complicated and does too many things
• Having the sweeper externally gives greater flexibility in implementing different sweep strategies.

I however, agree that it would be great to have better validation in register_parameter to error early if setpoints is a ParameterBase subclass. A pr for that is most welcome.

[thangleiter]

Ah, apologies for not searching for an existing issue. I don't actually use the functionality, so I'm all for keeping Parameter simple.

I can open up a PR for validation in register_parameter, but it's only putting out one small fire compared to the proper solution in #5036.

[jenshnielsen]

@thangleiter I think we should reopen since we have only resolved this partially? Do you agree

[thangleiter]

@thangleiter I think we should reopen since we have only resolved this partially? Do you agree

I guess it makes sense to keep it open for bookkeeping. The last of the examples above is indeed not fixed by #6084.

[jenshnielsen]

Thinking a bit more about this my proposed solution is.

• Add sweep and __getitem__ above to a new Sweeper class in qcodes-loop and update all of that pacakge to make use of this.
• Cut a new release of qcodes-loop
• Deprecate these two methods