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

Move tasks dependencies to fractal-tasks extra #378

Closed
jluethi opened this issue May 31, 2023 · 6 comments · Fixed by #390
Closed

Move tasks dependencies to fractal-tasks extra #378

jluethi opened this issue May 31, 2023 · 6 comments · Fixed by #390
Labels
High Priority Current Priorities & Blocking Issues package

Comments

@jluethi
Copy link
Collaborator

jluethi commented May 31, 2023

Goal: Split the dependencies into the library dependencies (needed to use all the lib functionality like ROI processing, the wrapper to run a task, channel handling, masked loading etc.) and the actual core tasks (e.g. depending on cellpose, napari, napari plugins etc.)

Just installing fractal-tasks-core should be somewhat lightweight, so that other packages like faim-hcs, scMultipleX, the OME-Zarr ROI loading plugin etc. can depend on it for the library functionality.

If the extra "fractal-tasks" is added, then also dependencies for the core tasks are installed, e.g. cellpose.

To collect the tasks in the future, that extra will need to be added.

@tcompa tcompa mentioned this issue May 31, 2023
@tcompa tcompa added package High Priority Current Priorities & Blocking Issues labels May 31, 2023
@tcompa
Copy link
Collaborator

tcompa commented Jun 6, 2023

I think this should also come with a clear separation in terms of subpackages.

I list two options below; I have a preference for option B, but I'm OK with any of the two.

Option A

fractal_tasks_core
|--- task1.py
|--- task2.py
|--- dev
|    |--- tool1.py
|    |--- tool2.py
|--- lib
|    |--- lib1.py
|    |--- lib2.py

Imports would then look like

from fractal_tasks_core.create_ome_zarr import create_ome_zarr
from fractal_tasks_core.lib.lib1 import function
from fractal_tasks_core.dev.tool1 import another_function

Note that import verbosity does not really increase, here, because after having a separate lib subpackage we can strip the lib_ prefix from the names of Python files.

Option B

In fact the most logical option would be a different one, where the main package is used for the lib functions, and we move tasks to their own subpackage:

fractal_tasks_core
|--- lib1.py
|--- lib2.py
|--- dev
|    |--- tool1.py
|    |--- tool2.py
|--- tasks
|    |--- task1.py
|    |--- task2.py

So that imports look like

from fractal_tasks_core.tasks.create_ome_zarr import create_ome_zarr
from fractal_tasks_core.lib1 import function
from fractal_tasks_core.dev.tool1 import another_function

This would be the most logical option, because lib functions are the ones that are guaranteed to work (in terms of dependencies) when the package is installed without the tasks extra.

The (minor) downside here is that we need to review an assumption we rely on in fractal-server, namely that tasks (and their manifest, or whatever additional files we'll introduce in the future) are all in the main package. Fixing this won't require too much work, but let's keep in mind that then it will be required also for other packages.

Also note that this may add a bit of import verbosity for those who want to use the tasks as functions, but that seems acceptable.

@jluethi
Copy link
Collaborator Author

jluethi commented Jun 6, 2023

In fact the most logical option would be a different one, where the main package is used for the lib functions, and we move tasks to their own subpackage:

I'm in favor of this! Depending on the package will mostly be done for the core library functions, the tasks are their own sub package.
Also, it's how I've been adding Fractal tasks to other packages like scMultipleX & faim-hcs now.

The (minor) downside here is that we need to review an assumption we rely on in fractal-server, namely that tasks (and their manifest, or whatever additional files we'll introduce in the future) are all in the main package. Fixing this won't require too much work, but let's keep in mind that then it will be required also for other packages.

My experience with scMultipleX & faim-hcs: Currently, the manifest needs to be in the main folder, but the individual tasks can be in their own subpackage. See example here: https://github.com/jluethi/faim-hcs/tree/fractal-roi-tables/src/faim_hcs
The executables then just need to be specified correctly in the task manifest.
We can discuss whether the manifest should also be put into the sub-package, but I don't think it's a big issue to have that in the main package folder.

@tcompa
Copy link
Collaborator

tcompa commented Jun 6, 2023

OK, thanks for the faim example.

We can discuss whether the manifest should also be put into the sub-package, but I don't think it's a big issue to have that in the main package folder.

Minor issue, I agree. Let' leave it where it is, for the moment.

EDIT: the reason why it's a minor issue is that the manifest itself is not associated to any dependency. It's just one more file in the package, which could also be seen as a piece of metadata.

@tcompa
Copy link
Collaborator

tcompa commented Jun 6, 2023

Concerning the name of the new extra (ref #390 (comment)), it is OK to have fractal-tasks.

Note that for the moment we would not be able to use fractal_tasks, because of pip being out of date with respect of PEP685 (and poetry being up-to-date):

(also ref fractal-analytics-platform/fractal-server#736)

@tcompa tcompa changed the title Making task dependencies optional Move tasks dependencies to fractal-tasks extra Jun 6, 2023
@tcompa
Copy link
Collaborator

tcompa commented Jun 6, 2023

For the record, installing the core package in a fresh venv takes about 360M, while adding the fractal-tasks extra leads to a venv-folder size of 2.8G.

@jluethi
Copy link
Collaborator Author

jluethi commented Jun 6, 2023

For the record, installing the core package in a fresh venv takes about 360M, while adding the fractal-tasks extra leads to a venv-folder size of 2.8G.

This is awesome! That makes it very encouraging to depend on fractal-tasks-core core package in a few places then (the new tasks I'm writing, but also the napari-ome-zarr-roi-loader plugin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High Priority Current Priorities & Blocking Issues package
Projects
None yet
2 participants