Refactor core-library modules

[tcompa]

Ref #600

Checklist before merging

• I added an appropriate entry to CHANGELOG.md

[github-actions]

Coverage report

The coverage rate went from 90.78% to 90.8% ⬆️
The branch rate is 84%.

99.07% of new lines are covered.

Diff Coverage details (click to unfold)

fractal_tasks_core/init.py

100% of new lines are covered (100% of the complete file).

fractal_tasks_core/cellvoyager/init.py

100% of new lines are covered (100% of the complete file).

fractal_tasks_core/cellvoyager/filenames.py

100% of new lines are covered (97.22% of the complete file).

fractal_tasks_core/channels.py

100% of new lines are covered (98.25% of the complete file).

fractal_tasks_core/labels.py

100% of new lines are covered (100% of the complete file).

fractal_tasks_core/masked_loading.py

100% of new lines are covered (82.89% of the complete file).

fractal_tasks_core/ngff/init.py

100% of new lines are covered (100% of the complete file).

fractal_tasks_core/ngff/specs.py

100% of new lines are covered (95.29% of the complete file).

fractal_tasks_core/ngff/zarr_utils.py

93.87% of new lines are covered (94.33% of the complete file).
Missing lines: 77, 78, 82

fractal_tasks_core/roi/v1_checks.py

100% of new lines are covered (100% of the complete file).

fractal_tasks_core/roi/v1_overlaps.py

100% of new lines are covered (94.68% of the complete file).

fractal_tasks_core/tables/init.py

100% of new lines are covered (100% of the complete file).

fractal_tasks_core/tables/v1.py

100% of new lines are covered (97.76% of the complete file).

fractal_tasks_core/tasks/apply_registration_to_ROI_tables.py

100% of new lines are covered (90.17% of the complete file).

fractal_tasks_core/tasks/apply_registration_to_image.py

100% of new lines are covered (78.8% of the complete file).

fractal_tasks_core/tasks/calculate_registration_image_based.py

100% of new lines are covered (90% of the complete file).

fractal_tasks_core/tasks/cellpose_segmentation.py

100% of new lines are covered (85.86% of the complete file).

fractal_tasks_core/tasks/copy_ome_zarr.py

100% of new lines are covered (89.32% of the complete file).

fractal_tasks_core/tasks/create_ome_zarr.py

100% of new lines are covered (82.9% of the complete file).

fractal_tasks_core/tasks/create_ome_zarr_multiplex.py

100% of new lines are covered (87.73% of the complete file).

fractal_tasks_core/tasks/illumination_correction.py

100% of new lines are covered (82.01% of the complete file).

fractal_tasks_core/tasks/import_ome_zarr.py

100% of new lines are covered (85.27% of the complete file).

fractal_tasks_core/tasks/maximum_intensity_projection.py

100% of new lines are covered (89.28% of the complete file).

fractal_tasks_core/tasks/napari_workflows_wrapper.py

100% of new lines are covered (90.33% of the complete file).

fractal_tasks_core/tasks/napari_workflows_wrapper_models.py

100% of new lines are covered (93.02% of the complete file).

fractal_tasks_core/tasks/yokogawa_to_ome_zarr.py

100% of new lines are covered (91.37% of the complete file).

fractal_tasks_core/utils.py

100% of new lines are covered (97.91% of the complete file).

fractal_tasks_core/zarr_utils.py

100% of new lines are covered (97.43% of the complete file).

[tcompa]

For the record, here is (part of) the current package structure:

$ tree -L 2 -I tasks -I dev

.
├── __FRACTAL_MANIFEST__.json
├── __init__.py
├── ome_zarr
│   ├── channels.py
│   ├── __init__.py
│   ├── input_models.py
│   ├── masked_loading.py
│   ├── ngff.py
│   ├── pyramids.py
│   ├── read_fractal_metadata.py
│   ├── write.py
│   └── zattrs_utils.py
├── roi
│   ├── __init__.py
│   ├── load_region.py
│   ├── _overlaps_common.py
│   ├── v1_checks.py
│   ├── v1_overlaps.py
│   └── v1.py
├── tables
│   ├── __init__.py
│   └── v1.py
├── upscale_array.py
└── yokogawa
    ├── filenames.py
    ├── __init__.py
    └── metadata.py

4 directories, 23 files

[tcompa]

A first bunch of minor issues where I think we can still improve:

• in fractal_tasks_core.roi.is_ROI_table_valid, we validate both the ROI contents and the Zarr attributes --> should we split this into two functions, which belong either to roi or to tables subpackages? --> postponed
• should we rename fractal_tasks_core.ome_zarr into fractal_tasks_core.ngff? This would be mostly for import-statement length --> fixed by extracting ngff from ome_zarr
• Either way, should the current fractal_tasks_core.ome_zarr.ngff module become something like fractal_tasks_core.ome_zarr.specs or fractal_tasks_core.ome_zarr.metadata? Or, if we take the other option, something like fractal_tasks_core.ngff.specs or fractal_tasks_core.ngff.metadata?
• Can we avoid the import from a different subpackage that currently takes place in tables/v1.py? (as in from ..ome_zarr.write import _write_elem_with_overwrite)

[jluethi]

should we rename fractal_tasks_core.ome_zarr into fractal_tasks_core.ngff? This would be mostly for import-statement length

I'd make the following separation: Things that validate a json structure are ngff. Things that use the zarr API, e.g. to load zattrs or other zarr specific parts are ome-zarr.

In the (far) future, there might be other ngff backends besides zarr, which was the motivation to keep the specification abstract with its own ngff name

[tcompa]

Here is the current status. I think things are improving, but we may still clean up the structure a bit, especially in ome_zarr subpackage - which is the least well-defined one. I'll now look at the write.py location and scope.

$ tree -L 2 -I tasks -I dev fractal_tasks_core/ -I __pycache__

fractal_tasks_core/
├── cellvoyager
│   ├── filenames.py
│   ├── __init__.py
│   └── metadata.py
├── __FRACTAL_MANIFEST__.json
├── __init__.py
├── ngff
│   ├── __init__.py
│   ├── specs.py
│   └── zarr_utils.py
├── ome_zarr
│   ├── channels.py
│   ├── __init__.py
│   ├── input_models.py
│   ├── label_group.py
│   ├── masked_loading.py
│   ├── pyramids.py
│   ├── write.py
│   └── zattrs_utils.py
├── roi
│   ├── __init__.py
│   ├── load_region.py
│   ├── _overlaps_common.py
│   ├── v1_checks.py
│   ├── v1_overlaps.py
│   └── v1.py
├── tables
│   ├── __init__.py
│   └── v1.py
└── upscale_array.py

[jluethi]

Sounds good, great to see this becoming so structured! :)

[tcompa]

I think I'm mostly done with this PR, with the structure below. It clearly can still improve, but it's already quite a large refactor.

The last point I'd like to fix is the name of fractal_tasks_core/write.py, which is the module where we define:

1. The OverwriteNotAllowedError custom error
2. The low-level open_zarr_group_with_overwrite function, which wraps zarr.open_group by adding overwriting-related checks

Thus this is essentially a wrapper of (a very limited part of) the zarr-python API. This module (which could become a subpackage in the future) should host our own Zarr-related functionalities which are fully unrelated to NGFF or other Fractal-specific features.

I'm tempted to go with fractal_tasks_core.zarr. This is against my own advice about the risk of overlap between from .zarr and from zarr, although I can partly take it back since from zarr import open_zarr_group_with_overwrite would always fail (while the correct form could be from .zarr import open_zarr_group_with_overwrite, when accessing it from the same folder).

I'll now proceed with this one, and then that's mostly it from me here (cc @jluethi). If there are better suggestions on this name, they are welcome.

---

$ tree -L 2 -I tasks -I dev fractal_tasks_core/ -I __pycache__

fractal_tasks_core/
├── cellvoyager
│   ├── filenames.py
│   ├── __init__.py
│   └── metadata.py
├── __FRACTAL_MANIFEST__.json
├── __init__.py
├── labels.py
├── ngff
│   ├── __init__.py
│   ├── specs.py
│   └── zarr_utils.py
├── ome_zarr
│   ├── channels.py
│   ├── __init__.py
│   ├── input_models.py
│   ├── masked_loading.py
│   ├── pyramids.py
│   └── zattrs_utils.py
├── roi
│   ├── __init__.py
│   ├── load_region.py
│   ├── _overlaps_common.py
│   ├── v1_checks.py
│   ├── v1_overlaps.py
│   └── v1.py
├── tables
│   ├── __init__.py
│   └── v1.py
├── upscale_array.py
└── write.py

[jluethi]

Big fan of how cellvoyager, rois & tables are separate. And generally of the additional structure added.

The 2 questions I'd like to briefly review tomorrow:

• ngff vs. ome-zarr split in functionality
• Do we have a top-level zarr.py and an ome-zarr module? The zarr.py functionality is unrelated to the ome-zarr module?

[tcompa]

Latest structure:

fractal_tasks_core/
├── cellvoyager
│   ├── filenames.py
│   ├── __init__.py
│   └── metadata.py
├── ngff
│   ├── __init__.py
│   ├── specs.py
│   └── zarr_utils.py
├── roi
│   ├── __init__.py
│   ├── load_region.py
│   ├── _overlaps_common.py
│   ├── v1_checks.py
│   ├── v1_overlaps.py
│   └── v1.py
├── tables
│   ├── __init__.py
│   └── v1.py
├── tasks
│   ├── apply_registration_to_image.py
│   ├── apply_registration_to_ROI_tables.py
│   ├── calculate_registration_image_based.py
│   ├── cellpose_segmentation.py
│   ├── compress_tif.py
│   ├── copy_ome_zarr.py
│   ├── create_ome_zarr_multiplex.py
│   ├── create_ome_zarr.py
│   ├── illumination_correction.py
│   ├── import_ome_zarr.py
│   ├── __init__.py
│   ├── maximum_intensity_projection.py
│   ├── napari_workflows_wrapper_models.py
│   ├── napari_workflows_wrapper.py
│   ├── _utils.py
│   └── yokogawa_to_ome_zarr.py
├── __FRACTAL_MANIFEST__.json
├── __init__.py
├── channels.py
├── labels.py
├── masked_loading.py
├── pyramids.py
├── upscale_array.py
├── utils.py
└── zarr_utils.py

Side note: napari_workflows_wrapper_models.py is needed (next to napari_workflows_wrapper.py) so that this file can be imported (to generate JSON Schemas) without relying on import napari_workflows (which may not be available, depending on the presence/absence of the pip extra).

[tcompa]

Here is a first version of the main import changes; note that I only tracked the one from within fractal_tasks_core/tasks.

I sorted them by original import, and I will now group them a bit to make it more readable (and then put this info somewhere in the docs).

-from fractal_tasks_core.lib_channels import ChannelNotFoundError
+from fractal_tasks_core.channels import ChannelNotFoundError

-from fractal_tasks_core.lib_channels import OmeroChannel
+from fractal_tasks_core.channels import OmeroChannel

-from fractal_tasks_core.lib_channels import check_unique_wavelength_ids
+from fractal_tasks_core.channels import check_unique_wavelength_ids

-from fractal_tasks_core.lib_channels import check_well_channel_labels
+from fractal_tasks_core.channels import check_well_channel_labels

-from fractal_tasks_core.lib_channels import define_omero_channels
+from fractal_tasks_core.channels import define_omero_channels

-from fractal_tasks_core.lib_channels import get_channel_from_image_zarr
+from fractal_tasks_core.channels import get_channel_from_image_zarr

-from fractal_tasks_core.lib_channels import get_omero_channel_list
+from fractal_tasks_core.channels import get_omero_channel_list

-from fractal_tasks_core.lib_channels import update_omero_channels
+from fractal_tasks_core.channels import update_omero_channels

-from fractal_tasks_core.lib_glob import glob_with_multiple_patterns
+from fractal_tasks_core.cellvoyager.filenames import glob_with_multiple_patterns

-from fractal_tasks_core.lib_input_models import Channel
+from fractal_tasks_core.channels import ChannelInputModel

-from fractal_tasks_core.lib_input_models import NapariWorkflowsInput
+from fractal_tasks_core.tasks.napari_workflows_wrapper_models import NapariWorkflowsInput

-from fractal_tasks_core.lib_input_models import NapariWorkflowsOutput
+from fractal_tasks_core.tasks.napari_workflows_wrapper_models import NapariWorkflowsOutput

-from fractal_tasks_core.lib_masked_loading import masked_loading_wrapper
+from fractal_tasks_core.masked_loading import masked_loading_wrapper

-from fractal_tasks_core.lib_metadata_parsing import parse_yokogawa_metadata
+from fractal_tasks_core.cellvoyager.metadata import parse_yokogawa_metadata

-from fractal_tasks_core.lib_ngff import NgffImageMeta
+from fractal_tasks_core.ngff import NgffImageMeta

-from fractal_tasks_core.lib_ngff import detect_ome_ngff_type
+from fractal_tasks_core.ngff import detect_ome_ngff_type

-from fractal_tasks_core.lib_ngff import load_NgffImageMeta
+from fractal_tasks_core.ngff import load_NgffImageMeta

-from fractal_tasks_core.lib_ngff import load_NgffWellMeta
+from fractal_tasks_core.ngff import load_NgffWellMeta

-from fractal_tasks_core.lib_parse_filename_metadata import parse_filename
+from fractal_tasks_core.cellvoyager.filenames import parse_filename

-from fractal_tasks_core.lib_pyramid_creation import build_pyramid
+from fractal_tasks_core.pyramids import build_pyramid

-from fractal_tasks_core.lib_read_fractal_metadata import get_parameters_from_metadata
+from fractal_tasks_core.utils import get_parameters_from_metadata

-from fractal_tasks_core.lib_regions_of_interest import are_ROI_table_columns_valid
+from fractal_tasks_core.roi import are_ROI_table_columns_valid

-from fractal_tasks_core.lib_regions_of_interest import array_to_bounding_box_table
+from fractal_tasks_core.roi import array_to_bounding_box_table

-from fractal_tasks_core.lib_regions_of_interest import check_valid_ROI_indices
+from fractal_tasks_core.roi import check_valid_ROI_indices

-from fractal_tasks_core.lib_regions_of_interest import convert_ROI_table_to_indices
+from fractal_tasks_core.roi import convert_ROI_table_to_indices

-from fractal_tasks_core.lib_regions_of_interest import convert_ROIs_from_3D_to_2D
+from fractal_tasks_core.roi import convert_ROIs_from_3D_to_2D

-from fractal_tasks_core.lib_regions_of_interest import convert_indices_to_regions
+from fractal_tasks_core.roi import convert_indices_to_regions

-from fractal_tasks_core.lib_regions_of_interest import empty_bounding_box_table
+from fractal_tasks_core.roi import empty_bounding_box_table

-from fractal_tasks_core.lib_regions_of_interest import find_overlaps_in_ROI_indices
+from fractal_tasks_core.roi import find_overlaps_in_ROI_indices

-from fractal_tasks_core.lib_regions_of_interest import get_image_grid_ROIs
+from fractal_tasks_core.roi import get_image_grid_ROIs

-from fractal_tasks_core.lib_regions_of_interest import get_overlapping_pairs_3D
+from fractal_tasks_core.roi import get_overlapping_pairs_3D

-from fractal_tasks_core.lib_regions_of_interest import get_single_image_ROI
+from fractal_tasks_core.roi import get_single_image_ROI

-from fractal_tasks_core.lib_regions_of_interest import is_ROI_table_valid
+from fractal_tasks_core.roi import is_ROI_table_valid

-from fractal_tasks_core.lib_regions_of_interest import is_standard_roi_table
+from fractal_tasks_core.roi import is_standard_roi_table

-from fractal_tasks_core.lib_regions_of_interest import load_region
+from fractal_tasks_core.roi import load_region

-from fractal_tasks_core.lib_regions_of_interest import prepare_FOV_ROI_table
+from fractal_tasks_core.roi import prepare_FOV_ROI_table

-from fractal_tasks_core.lib_regions_of_interest import prepare_well_ROI_table
+from fractal_tasks_core.roi import prepare_well_ROI_table

-from fractal_tasks_core.lib_regions_of_interest import remove_FOV_overlaps
+from fractal_tasks_core.roi import remove_FOV_overlaps

-from fractal_tasks_core.lib_tables import write_table
+from fractal_tasks_core.tables import write_table

-from fractal_tasks_core.lib_upscale_array import upscale_array
+from fractal_tasks_core.upscale_array import upscale_array

-from fractal_tasks_core.lib_write import OverwriteNotAllowedError
+from fractal_tasks_core.zarr_utils import OverwriteNotAllowedError

-from fractal_tasks_core.lib_write import open_zarr_group_with_overwrite
+from fractal_tasks_core.zarr_utils import open_zarr_group_with_overwrite

-from fractal_tasks_core.lib_write import prepare_label_group
+from fractal_tasks_core.labels import prepare_label_group

-from fractal_tasks_core.lib_zattrs_utils import get_table_path_dict
+from fractal_tasks_core.utils import get_table_path_dict

-from fractal_tasks_core.lib_zattrs_utils import rescale_datasets
+from fractal_tasks_core.utils import rescale_datasets

[tcompa]

I'm merging this one, and I'll give it a final review before releasing 0.14.0.

[jluethi]

Awesome, great that we have this core library refactor now!