imr-framework / pypulseq

Pulseq in Python
https://pypulseq.readthedocs.io
GNU Affero General Public License v3.0
125 stars 67 forks source link

Introduction of custom variables #199

Open schuenke opened 3 months ago

schuenke commented 3 months ago

For PR #198, I was thinking of adding a types.py to define a custom variable for (gradient) channels, but I decided to create an Issue first and hear about your opinions.

For (grad) channels, the custom var could simply look like this:

from typing import Literal

# Define custom type for gradient channels
Channel = Literal['x', 'y', 'z']

And of course there would be more useful custom vars, for example for labels ('SLC', 'SEG', 'REP' etc).

The advantages would be:

Let me know what you think about this.

FrankZijlstra commented 3 months ago

Is there a way to iterate over the values in a Literal? I think when defining something like this globally, it would be nice to then have a way to say: for c in Channel, or to use len(Channel)

With that in mind, wouldn't an Enum be a better type, in that it allows both those things? The only issue I see there is that all old code becomes incompatible because passing 'x' needs to be replaced with Channel.X...

Edit: Python 3.11 has StrEnum which seems to allow also using string values?

schuenke commented 3 months ago

You can easily get what you asked for using

import typing

Channel = typing.Literal["x", "y", "z"]

VALID_CHANNELS = typing.get_args(Channel)
print(f"Valid channels are: {VALID_CHANNELS}")
FrankZijlstra commented 3 months ago

I did a quick scan of the codebase, found the following literals that could be typed like this:

schuenke commented 2 months ago

Perfect. Lets leave this open and expand the list if we realize it's not complete yet. But maybe we can switch from the current flat-layout to the src-layout before introduction of the Literals to avoid problems like the one in #202. If we do both in parallel, merging will be a pain 🙄