DiamondLightSource / DiamondLightSource/httomo

Revaluate whether `StandardLoaderWrapper` is needed or not

Open
#504 0 comments 0 reactions 0 assignees View on GitHub
loader refactor
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.