Parameter.__getitem__ behavior
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 459
- Forks
- 359
- Avg merge
- 3d 6h
- Merged PRs (30d)
- 73
Description
`Parameter`s 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`:
```python
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:
```python
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:
```python
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with Parameter.__getitem__, Measurement.register_parameter, and _register_parameter_with_setpoints(), reproducing the obscure error and non-terminating iteration from the examples. Define the replacement and deprecation behavior, then add coverage showing invalid setpoints fail clearly and the supported sweep or indexing use case remains available.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api, backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100