DiamondLightSource / DiamondLightSource/httomo

Incorrect type of key in `PreviewParam` for representing a key that may not be present

Open
#608 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
10
Forks
5
Avg merge
5d 17h
Merged PRs (30d)
1

Description

`PreviewParam` is intended to represent the python dict that is the value of the `preview` parameter in a loader's config (if the `preview` parameter value is given at all, it can also be `None`):
https://github.com/DiamondLightSource/httomo/blob/584e15ccd84fbd7f440454b6d3c48a071b8f2f83/httomo/transform_loader_params.py#L48-L55

The intention in the definition of the type was that any of the keys could be *omitted*. However, due to the use of the `Optional` type, in its current state, the `PreviewParam` type states that:
- all three keys *must be present* in the dict
- the value of the keys can be either `None` or some other type

which isn't what was intended.

The `Optional` type alone is not the correct type for what is desired.

The [`NotRequired` type](https://typing.python.org/en/latest/spec/typeddict.html#required-and-notrequired) provides a piece of the puzzle.

However, `NotRequired[T]` isn't _quite_ correct either. This is because `NotRequired[T]` disallows setting the key's value to `None`, which _is_ allowed.

The correct type should allow:
- omitting of a key
- but also providing the key as either `None` or the `T`

which I think is accomplished with `NotRequired[Optional[T]]`.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.