DiamondLightSource / DiamondLightSource/httomo
Revaluate whether `StandardLoaderWrapper` is needed or not
- Dominant language
- Python
- Stars
- 10
- Forks
- 5
- Avg merge
- 5d 17h
- Merged PRs (30d)
- 1
Description
## TL;DR
- the runner wants to be able to get a loader generically, so it relies on any implementor of `LoaderInterface` to get one
- the `StandardLoaderWrapper` class implements `LoaderInterface`, so the runner uses `StandardLoaderWrapper` to get a loader
- the `StandardLoaderWrapper` class wraps the `StandardTomoLoader` class, so then the latter can be used by the runner
- the `StandardTomoLoader` class can be very slightly modified (adding three property-getters, `detector_x`, `detector_y`, `angles_total`) to implement `LoaderInterface`, to avoid the extra layer of `StandardLoaderWrapper`
- remove the `make_data_source()` method on the `LoaderInterface` protocol (which then means that `StandardTomoLoader` now would implement `LoaderInterface`)
- then, make the loader factory function `make_loader()` return an instance of `StandardTomoLoader` instead of `StandardLoaderWrapper`
## Details
The `Pipeline` object represents the loader and methods in the pipeline. In particular, for the loader, it requires anything that implements the `LoaderInterface` protocol: https://github.com/DiamondLightSource/httomo/blob/f6bb8aca8a4fb10cce5dd55c94dff2d0f1d77325/httomo/runner/pipeline.py#L12-L14
When the UI layer creates a `Pipeline` object, it creates an implementor of `LoaderInterface` by using the `make_loader()` function (which could be viewed as a factory function for implementors of `LoaderInterface`): https://github.com/DiamondLightSource/httomo/blob/f6bb8aca8a4fb10cce5dd55c94dff2d0f1d77325/httomo/ui_layer.py#L143-L159
The task runner then uses the `make_data_source()` method defined on the `LoaderInterface` protocol to generate an implementor of `DataSetSource`: https://github.com/DiamondLightSource/httomo/blob/f6bb8aca8a4fb10cce5dd55c94dff2d0f1d77325/httomo/runner/task_runner.py#L226
Now, the way that `StandardLoaderWrapper` implements `LoaderInterface` is _mostly_ by using the `StandardTomoLoader.global_shape` getter: https://github.com/DiamondLightSource/httomo/blob/f6bb8aca8a4fb10cce5dd55c94dff2d0f1d77325/httomo/loaders/standard_tomo_loader.py#L394-L421
Note that the three property getters:
- `detector_x`
- `detector_y`
- `angles_total`
are simply using `StandardTomoLoader.global_shape`, and that the `make_data_source()` method is implemented simply by creating a `StandardTomoLoader` instance.
What this means is that:
- if `LoaderInterface` only required the three property getters `detector_x`, `detector_y`, `angles_total`
- then `StandardTomoLoader` could easily be changed to implement `LoaderInterface`
- and the runner would be able to get access to an implementor of both `LoaderInterface` and `DataSetSource` from having an instance of `StandardTomoLoader`
Lastly, the `make_loader()` factory function could then simply create instances that implement both:
- `LoaderInterface`
- `DataSetSource`
and the runner would be able to get a data source without needing to use an intermediate "loader wrapper class" to generate a data source.
I think this could possibly make the "way" that the runner gets access to a data source simpler, and easier to understand, by getting rid of a layer in the generation of data source.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.