Refactor core-library modules

[tcompa]

lib_* prefix is a leftover from when core-library and tasks where in the same namespace, but I think it now just makes every import longer with no real benefit.

Such a change is not at all urgent, but I also think it will only break the packages that do depend on fractal-tasks-core (ref https://github.com/fractal-analytics-platform/fractal-tasks-core/issues/599) and then it should be fairly harmless.

[jluethi]

I’m fine with the idea, but let’s combine it with any other reorg needed. For example: do we want to make some core functions importable from the top level? Any other changes like reprg of the current lib files (are there any that should be merged or split?) would also make sense.

In that way, task packages then only need to update once.

Besides combining this, in favor of changing sooner rather than later! Because the number of packages may grow soon

[tcompa]

I’m fine with the idea, but let’s combine it with any other reorg needed.

Fully agreed.

My first guess is that we may refactor the tables-related package structure, based on the shape that V1 specs will take (#582).

For example: do we want to make some core functions importable from the top level?

I'm generally in favor. Let's find out for which functions it would make sense.

[tcompa]

Based on our call last week with @jluethi, here's a first proposal of how to refactor current core-library modules (I'm still trying to improve some details):

• fractal_tasks_core.ome_zarr: broad-scope subpackage with all what is related to ome-zarr
  • fractal_tasks_core.ome_zarr.channels
  • fractal_tasks_core.ome_zarr.input_models
  • fractal_tasks_core.ome_zarr.ngff -> we may want a better naming for this one
  • fractal_tasks_core.ome_zarr.masked_loading
  • fractal_tasks_core.ome_zarr.pyramids
  • fractal_tasks_core.ome_zarr.read_fractal_metadata -> we may want a better name
  • fractal_tasks_core.ome_zarr.write -> split this into two parts, with one specifically related to labels
  • fractal_tasks_core.ome_zarr.utils -> if possible, let's find a more specific name
• fractal_tasks_core.yokogawa -> subpackage with all helper functions which are specific for the Yokogawa converted (@jluethi: is this the name we want to use?)
  • fractal_tasks_core.yokogawa.glob -> rename it to avoid conflicts with glob library (best candidate: filenames)
  • fractal_tasks_core.yokogawa.filename_metadata -> merge with filenames
  • fractal_tasks_core.yokogawa.metadata_parsing
• fractal_tasks_core.upscale_array: single module
• fractal_tasks_core.roi: subpackage with all what is currently in lib_regions_of_interest
• fractal_tasks_core.tables: subpackage related to tables, in principle unrelated to the fact that we use tables to store ROIs of Zarr arrays

[tcompa]

Some relevant constraints that we can try to enforce with the new structure (and which are valid as of https://github.com/fractal-analytics-platform/fractal-tasks-core/pull/631#issuecomment-1850155595):

• __FRACTAL_TABLE_VERSION__ is only used within fractal_tasks_core.table
• __OME_NGFF_VERSION__ is only used within fractal_tasks_core.ome_zarr and fractal_tasks_core.tasks
• zarr is only imported within fractal_tasks_core.ome_zarr and fractal_tasks_core.tasks; this is not strictly true, since it leaks into:
  • fractal_tasks_core.roi.is_ROI_table_valid, where we validate both the ROI contents and the Zarr attributes --> maybe we can refactor this one?
  • in fractal_tasks_core.tables (but only for type hints)
• we don't import from ..: this is currently broken in tables, by from ..ome_zarr.write import _write_elem_with_overwrite

[jluethi]

fractal_tasks_core.yokogawa -> subpackage with all helper functions which are specific for the Yokogawa converted (@jluethi: is this the name we want to use?)

Let's go with fractal_tasks_core.cellvoyager, because Yokogawa is the broad brand, we're only covering their cellvoyager microscopes

fractal_tasks_core.yokogawa.glob -> rename it to avoid conflicts with glob library (best candidate: filenames)

Let's go with fractal_tasks_core. cellvoyager.glob

fractal_tasks_core.ome_zarr.ngff -> we may want a better naming for this one

Can we describe the scope of this part?

fractal_tasks_core.ome_zarr.read_fractal_metadata -> we may want a better name

Can we describe the scope of this part?

Some relevant constraints that we can try to enforce with the new structure

Nice constraints!

fractal_tasks_core.roi.is_ROI_table_valid, where we validate both the ROI contents and the Zarr attributes --> maybe we can refactor this one?

Not necessary for the 0.14.0 release. Who should be checking the attrs in the end? I can see that it would come from a function in fractal_tasks_core.tables to load the attrs, but we also didn't really want to have it there, right?

we don't import from ..: this is currently broken in tables, by from ..ome_zarr.write import _write_elem_with_overwrite

+1!

[jluethi]

Not necessary for the 0.14.0 release. Who should be checking the attrs in the end? I can see that it would come from a function in fractal_tasks_core.tables to load the attrs, but we also didn't really want to have it there, right?

I guess it could come from the proposed read_table function :) https://github.com/fractal-analytics-platform/fractal-tasks-core/issues/632

[tcompa]

fractal_tasks_core.yokogawa.glob -> rename it to avoid conflicts with glob library (best candidate: filenames)

Let's go with fractal_tasks_core. cellvoyager.glob

I already moved to fractal_tasks_core.cellvoyager.filenames, since the two legacy modules (the one for filename parsing and the one for globbing) are somewhat related (as in "they both act on filenames of CellVoyager-generated image files, either by parsing of by filtering them").

(side remark: I'd tend to avoid using glob.py, because of the confusion between from .glob and from glob.)

[tcompa]

The main refactor result is now described at https://fractal-analytics-platform.github.io/fractal-tasks-core/version_updates/v0_14_0/.