DiamondLightSource / DiamondLightSource/blueapi
Replace Union handling for Plan parameters with more JSONSchema focused approach
- Dominant language
- Python
- Stars
- 13
- Forks
- 13
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 25
Description
Currently in `context.py._convert_type`, there is special Type Hint handling to provide syntactic sugar for command line environments so that users don't have to type brackets to specify a List of length one. e.g if they are specifying `delay `in the `count` plan they can type either
`delay=10`
or
`delay=[10, 30, 40]`
depending on whether they are specifying a single or multiple delays, and the type checking will accept the value without brackets for the single value case. This is because `_convert_type` specifies the type of `delay` as `Union[float | List[float]]` which allows both cases to pass type checking. Whilst this is good for command line cases, it is not for all others e.g. JSONSchema as it introduces unnecessary complexity into the schema that Pydantic produces, as internally `delay` is really just a `List[float]`, which is what the schema should contain.
This pattern of `Union[, List[] `applies to several base plans like `count` via `wrapped.py` in [dls-bluesky-core](https://github.com/DiamondLightSource/dls-bluesky-core/blob/main/src/dls_bluesky_core/plans/wrapped.py).
After discussion with Callum we have agreed that it is better to change the signature of the wrapped plans that this affects to just use raw `List`s for these parameters and move them to dodal, so that dls-bluesky core can be deleted. We have not however agreed on what to do next, so here are the options for discussiom:
### Keith's suggestion:
- For each affected Bluesky plan, retain a single instance of the wrapped plan that can be used for both command line and other use cases which has `List` parameters presented as just a `List`, not a `Union[, List[] `
- Copy the code that currently 'Unionizes' `List`s to the BlueAPI CLI parsing code where it can then apply the same change to `List` parameters, preserving the current command line experience of not having to type brackets for a list of 1 thing but only for the Command line case
- Probably retain the 'Unionizing' code in `_convert_type` for the time being as a means of identifying plans with parameters that should just be a `List` not `Union[, List[] `
**Advantages:**
- There is only one wrapped plan per source plan
- The non-command line case (e.g. JSONSchema consumption) works transparently as the parameter really is a `List`
- The command line case works transparently whether you type `` or `[]` as before and as it does when using raw bluesky without typechecking
**Complications**
In most cases the parameters that fall into this category are actually Optional, which means the are already actually Union[ | NoneType] so any handling would need to take acount of this too.
**Motivation**
- Presrving existing usage experience (Bluesky and current BlueAPI CLI
- To me, when the CLI presents/validates is just a VIew like in MVC which is tailored to the consumers of that view
### Callum's suggestion:
- Create an instance of a blueapi plan that does precisely what you want, and takes precisely the parameters you want with a particular interface in mind. E.g. if it's primarily for a GUI client probably don't include unnecessary syntactic sugar unions, if it's for a CLI client then do.
- Follow the pattern of having a core plan with all the complex logic and many small wrapper plans which take parameters tailored to a specific client. E.g. have a core scan plan and then at least two separate wrapper plans depending on whether the user is expecting to use a GUI vs a CLI.
- Do not touch any code in `_convert_type` or the CLI, pass the plan schema straight through from the python function to the client with as little fiddling/obfuscation as possible.
**Advantages:**
- There is one "thing" to edit, plans, that will have consequences for the client, which makes things easier for a scientist to understand.
- There is no code making extra, hidden changes to the plan schema that user may be unaware of.
- A GUI is harder to change than a CLI, even an autogenerated one, if you only expect your plan to change often, just use it with a CLI client and rapidly iterate it. If it's more stable, it can be used with the GUI.
**Complications**
- Likely to end up with many small, purpose-specific wrapper stubs that ultimately do the same thing.
**Motivation**
- Simplifying plan developer experience on a beamline
- While this will lead to more small, specific plans, bluesky is designed to make that cheap and easy via composability.
Contributor guide
Assessment
This issue has not been assessed yet.