Update tasks to Fractal 2.0

[jluethi]

Closes #669 Closes #682 Closes #324 Closes #415 Closes #299 Closes #523 Closes #535 Closes #686 Closes #674 Closes #691

Discussion

To what degree are we giving deprecation warnings or just changing some of the fractal-tasks-core functions? In general, public functions should stay stable. But we probably have some functions that shouldn't be public functions. For example, utils.get_table_path_dict: I'm now updating it to follow the new behavior. But also: zarr_url vs. zarrurl (e.g. in pyramid building) and other functions. I would expect this to become the 1.0 release of fractal-tasks-core. We have some other libraries depending on it, but not too many yet. Going forward, we need to take care not to break any public functions. This may be a good time to make some of those functions private now though.

To review

• [x] Using zarr_url instead of path (see https://github.com/fractal-analytics-platform/fractal-tasks-core/issues/670)

• [ ] Task output for apply_registration_to_image:

  • [ ] How are the image_list_updates structured?
  • [ ] Do image_list_updates now always provide the new image types or are they still applied based on filters set in the manifest? Edited images: An image zarr_url is provided that is already in the image => start from previous properties, gets updated with the manifest-based new filters (=> new types) Should manifest types be applied to input zarr_urls by default even if no image_list_update output is provided?

• [x] Task output in compound tasks: Can tasks provide custom Pydantic models as output? e.g. for the Cellvoyager converter: Can it produce InitArgsCellVoyager objects as output or just dicts that can be cast to that model in the input of the compute task? We can use the models to validate the dict we're building in the init task, but still output a dict in the init task Potential complexity argues for handling it in the task (=> more control to task developer). => We stay with dict output for init tasks (in init_args)

General recommendation: Create a dict, optionally validate it with a model

• [ ] In the image list outputs, can tasks provide dictionary items with value None? [for output for cellvoyager converter: Can I always provide the acquisition as an attribute even if it's None?] => That scenario doesn't matter, it was just init_args. If compute task output contains None => unsetting would make sense. A task should be able to unset an attribute. Unsetting via providing "my_attr": None Currently is an error.

Handling on the task side: Avoid putting in None when (exclude_Nones, exclude_unset) => How do we handle the complexities in InitArgs? In general, try to keep InitArgs simple

In general, the less hidden behaviors the better! We need to document any hidden behaviors or ways of doing the same thing in 2 ways.

• [ ] In the MIP task, I provide both a new type ("is_3D" = False) as well an origin that points to images with is_3D=True. Validate in the runner that the server then has is_3D=False once they ran

Cleanup

• [ ] Can fractal_tasks_core.utils.get_parameters_from_metadata be deprecated?
• [ ] Build pyramid uses zarrurl vs. zarr_url
• [ ] Remove old create_ome_zarr & yokogawa_to_ome_zarr functions

Checklist before merging

• [ ] I added an appropriate entry to CHANGELOG.md

[github-actions[bot]]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
fractal_tasks_core
channels.py
utils.py
fractal_tasks_core/ngff
specs.py
zarr_utils.py					98-104, 108-113
fractal_tasks_core/tables
v1.py					290, 317-324
fractal_tasks_core/tasks
_registration_utils.py					62, 209
_utils.py
apply_registration_to_image.py					163
calculate_registration_image_based.py
cellpose_segmentation.py					369
cellvoyager_to_ome_zarr_compute.py					208
cellvoyager_to_ome_zarr_init.py
cellvoyager_to_ome_zarr_init_multiplex.py					139
copy_ome_zarr_hcs_plate.py					231, 295-297
find_registration_consensus.py					106, 118, 166-168
illumination_correction.py					146, 273
image_based_registration_hcs_init.py					92-94
import_ome_zarr.py					212
init_group_by_well_for_multiplexing.py					61, 86-88
io_models.py
maximum_intensity_projection.py					149-162
napari_workflows_wrapper.py
Project Total

This report was generated by python-coverage-comment-action

[jluethi]

An interesting observation: Using the new zarr_url scheme, we can drop Path as an import from many of these tasks => promising for the direction of having cloud-compatible tasks! :)

[jluethi]

Should we rename the type "3D" to "is_3D" or something similar? To avoid having to switch to different dictionary approach

[jluethi]

I rewrote the Copy OME-Zarr task from scratch and changed the work distribution between copy & MIP parts => now more happens in the compute (MIP part). The copy only sets up the plate & well with the correct metadata. But in the new version, it's now explicitly specific for HCS plates, validates HCS metadata and writes the plate just with the wells that were passed with the zarr_urls (=> should open up for making this copy task an easy route for subset testing).

Having to handle the metadata of the OME-Zarrs lead to me introducing a few new Plate spec pydantic classes & copying over some of my helper functions from the navigator project (see https://github.com/fractal-analytics-platform/fractal-tasks-core/issues/678). Will be good to review which parts of those we want in Fractal-tasks-core.

One thing I'd like to review is whether this is the intended & simplest image_list_update we can provide:

    image_list_update_dict = dict(
        image_list_updates=[
            dict(
                zarr_url=zarr_url,
                origin=init_args.origin_url,
                types=dict(
                    is_3D=False,
                ),
            )
        ]
    )

Having to provide a dict of a list of a dict seems kind of cumbersome. I understand why it needs to be a list of dicts, as some tasks will provide updates to multiple images. Having this list in a dict with the only key image_list_updates is there so we know whether a task provided an image_list_updates or a parallelization_list I assume?

[jluethi]

@tcompa I need to build the new manifest tomorrow. Besides that, I think the tasks-core package would be mostly ready for examples 01 & 02.

Importing & multiplexing registration are still TBD, as well as a long list of tests & points above of course :)

[jluethi]

@tcompa I now added the draft task list. Maybe I'll find some time this afternoon to work on import & registration some more. Otherwise, this should be ready for testing manifest building & then also for running example 01 as far as I can say. (I'm sure we'll discover some bugs in the process, but the tasks follow the new API and the tests are updated for them at least)

[jluethi]

Closes #674

[jluethi]

I made the Find Registration Consensus a compound task now to provide an example of parallelizing over a well, which may be needed in some of Adrian's tasks quite often

[jluethi]

@tcompa I have an initial version of all the tasks ready now. The MIP task uses the old approach to get attributes again. The tests are passing, even one that runs cellpose after MIP, but I'll be curious to see if example 01 runs. I'll try to test that tomorrow morning :)

There are a few remaining parts on activating testing functionality, could you have a look at those?

• [x] Update & reenable manifest validation in Github CI
• [x] Update & reenable test_valid_args_schemas
• [x] Update test_task_interface in test_valid_task_interface once new Manifest is built

Other than that, the updates for manifest creation to make it slightly more flexible can either be part of this PR or a future PR. And I've create new issues for all the future goals in #669 .

[tcompa]

There are a few remaining parts on activating testing functionality, could you have a look at those?

Update & reenable manifest validation in Github CI Update & reenable test_valid_args_schemas Update test_task_interface in test_valid_task_interface once new Manifest is built

This are now covered, as of #689.

[tcompa]

I'll be curious to see if example 01 runs. I'll try to test that tomorrow morning

The cellpose task now runs:

Note that this test is currently a bit cumbersome, until we merge into main and make a pre-release.

One way is to use fractal-containers.

Another way is to run poetry build from the root directory of the fractal-tasks-core repo (within the V2_tasks branch), so that a wheel file is generated in the dist folder (and can be used for local-wheel task collection in fractal-web/server).

[tcompa]

Also napari-workflows runs, in fractal-demos (I did not check the output):

[jluethi]

Awesome!

Another way is to run poetry build from the root directory of the fractal-tasks-core repo (within the V2_tasks branch), so that a wheel file is generated in the dist folder (and can be used for local-wheel task collection in fractal-web/server).

Yes, also just setting up tests locally with server 2.0.0a4 and building the task package locally with python -m build :)

[jluethi]

I can confirm that I can also run example V1! =D

From my side, ready to merge after your review then. And I'll start exploring other examples and will open new issues where they occur :)

[tcompa]

Note: I pushed 34faa92e35f717a4a0060a2192ed31b201676125, with these changes:

Improve plate-related ngff models:

* Add `AcquisitionInPlate.description`;
* Make `AcquisitionInPlate.id` required;
* Add `ColumnInPlate`;
* Add `RowInPlate`.

[tcompa]

While reviewing the copy_ome_zarr_hcs_plate task, some parts (like the metadata generation) were a bit hard to understand. As a minor fix, I started using the load helper functions (including a new load_NgffPlateMeta one) and added a few comments - see this commit .

In the future this task would benefit from yet another review.

[jluethi]

Thanks a lot for the review & the fixes @tcompa !