Ngio projection

[lorenzocerrone]

• Add ngio to dependecies
• Refactor projection task to use ngio

Checklist before merging

• I added an appropriate entry to CHANGELOG.md

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
fractal_tasks_core/tables
v1.py
fractal_tasks_core/tasks
io_models.py
projection.py					76
Project Total

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

[jluethi]

This is great @lorenzocerrone !

A few brief questions: Have you tried running this projection task on a zyx dataset (no channel or time dimension)? Could we easily add a synthetic test case for that using an ngio generated tiny zyx OME-Zarr. That then closes #840

Also, would we expect this task to already work with time-data, e.g. an tczyx dataset?

How much we should test in tasks-core?
We test separately in tasks core only when there is task logic specifically to those cases

Can we add an option to copy over additional tables (e.g. user can deactivate that, but we copy non-roi tables by default). We'd keep them & ignore edge-cases (like missing label images) for now. TODO Joel: provide example dataset of condition tables for this.
Approach: First copy them as they are (by adding a include_table flag to the derive function or by adding a copy_tables function). Then modify the ROI tables in place in the new Zarr => let's go with a copy_tables function

TODO for myself:

• Check how we need to provide the metadata update for Review image-plate attribute of MIP task #841. Likely a change to the image_list_update_dict
• Run example 01 with this branch, to check for Update zenodo dataset, setting channel.active to True #823

[tcompa]

Naive question by someone who is not involved in ngio development/integration: where does this feature fit best?

Would it make sense to have something like

ngio.copy_tables(original_ngff_image, new_ngff_image, project_z=True)

or at least

ngio.copy_tables(original_ngff_image, new_ngff_image)
# and then set `z_length` manually

or would it be just additional complexity?

[lorenzocerrone]

The responsability of copying the tables is now moved to the NgffImage.derive_new_image method.

[tcompa]

Thanks @lorenzocerrone!
I added a few small comments, but the pattern is often the same (I am no expert in ngio, and then questions pop up frequently independently on the actual PR). I think we are at a stage where it would be useful to briefly discuss this live.

[tcompa]

I confirm that setting both origin and attributes/plate in the output is valid, and the value which will take priority is the one in attributes/plate.

[jluethi]

I added the logic now for updating the plate name and verified this by running the tasks in the Fractal containers. It now works as expected, the MIP Zarrs have a different plate name and depending on the plate name one selects, the correct plate viewer is shown with the vizarr plate loading.
Also closed a few minor issues here that were raised by @tcompa .

Thus, all my checks for this PR are done. @lorenzocerrone I summarize here what still remains before this can be merged, so hopefully it will be easier to keep an overview over that afterwards :)

• Switch back to using the task logger for the task & only use ngio logger for ngio activities
• Optionally update the task logger to improve its format
• Run a test on a (synthetic) zyx & tczyx dataset to see if things work as expected (no need to include new test cases for it unless we need to include special handling in the actual task code)
• Copy over additional tables as well, optionally with a warning for feature_tables & masking_roi_tables. Don't apply projection to non-roi tables, just copy the tables. Approach: Copy over all tables as part of the derive, then modify the roi tables to apply a projection? The copying can then be part of ngio, while the projection-specific operation is part of the task.
• Remove the if TYPE_CHECKING: approach for the time being?

[lorenzocerrone]

• Optionally update the task logger to improve its format

I think this should be handled in a separate PR

[lorenzocerrone]

The new version of the task has been tested with:

• tczyx
• tzyx
• czyx
• zyx

[lorenzocerrone]

@tcompa the PR is ready for review, but there is an error in the coverage CI. I am not sure what the issue is. Could you please give it a look?