Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loader factory function doesn't use MethodRepository or module_path params #533

Open
yousefmoazzam opened this issue Dec 11, 2024 · 0 comments
Labels

Comments

@yousefmoazzam
Copy link
Collaborator

This is something to be aware of if there's any discussions around where new loaders should be put (if that is ever a thing, to add new loaders to httomo).

Currently, the make_loader() function that's used to create instances of LoaderInterface implementors takes in

  • an implementor of MethodRepository
  • a module path

which it doesn't actually use:

def make_loader(
repo: MethodRepository,
module_path: str,
method_name: str,
in_file: Path,
data_path: str,
image_key_path: Optional[str],
angles: AnglesConfig,
darks: DarksFlatsFileConfig,
flats: DarksFlatsFileConfig,
preview: PreviewConfig,
comm: MPI.Comm,
) -> LoaderInterface:
"""
Factory function for creating implementors of `LoaderInterface`.
Notes
-----
Currently, only `StandardLoaderWrapper` is supported (and thus only `StandardTomoLoader` is
supported). Supporting other loaders is a topic that still needs to be explored.
See Also
--------
standard_tomo_loader.StandardLoaderWrapper : The only supported loader-wrapper
standard_tomo_loader.StandardTomoLoader : The only supported loader
"""
if "standard_tomo" not in method_name:
raise NotImplementedError(
"Only the standard_tomo loader is currently supported"
)
return StandardLoaderWrapper(
comm=comm,
in_file=in_file,
data_path=data_path,
image_key_path=image_key_path,
darks=darks,
flats=flats,
angles=angles,
preview=preview,
)

Relevant questions are things like:

  • would we ever need methods database info to create a loader (an implementor of LoaderInterface)
  • if we have different loaders, would they all be put into one module in httomo (in which case a module path parameter may not be needed), or would different loaders potentially be split into different modules (in which case a module path parameter may be useful)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant