ome / omero-scripts

Core OMERO Scripts
https://pypi.org/project/omero-scripts/
12 stars 32 forks source link

Add __init__.py files, use find #205

Closed pwalczysko closed 1 year ago

pwalczysko commented 1 year ago

Coming back to the idea of using the find_packages() of @will-moore from https://github.com/ome/omero-scripts/pull/204#issuecomment-1310452814, this PR:

  1. implements the find_packages() idea
  2. adds the __init__.py files in two places (The absence of these files was the reason why the idea ad. 1 above did not work in https://github.com/ome/omero-scripts/pull/204)

The reason for opening this PR is that:

  1. it will make missing of new folders in scripts by the setup.py harder in future (the only thing to remember is to add the init.py file into the new folder, which seems like a good practice anyway, as the previously created folders all have their init.py files)
  2. adds the init.py to the new annotation_scripts folder (which seems to be a good practice as stated in previous point)

Tested locally running

python setup.py build

and works fine.

cc @jburel @sbesson @will-moore

sbesson commented 1 year ago

The primary thing I would be very careful about and propose to be assessed is the usage of the overloaded omero namespace. If this repository starts declaring omero/__init__.py, is there some unexpected interplay with omero-py?

For reference, during earlier work on the extensibility of the OMERO CLI, the working pattern was to store the plugin classes under omero/plugins and explicitly declare it in setup.py - see https://github.com/ome/cookiecutter-omero-cli-plugin/blob/main/omero-%7B%7Bcookiecutter.cli_command%7D%7D/setup.py#L37 for the template or https://github.com/ome/omero-cli-duplicate/blob/master/setup.py#L99 and https://github.com/ome/omero-metadata/blob/master/setup.py#L100 for real-world examples.

pwalczysko commented 1 year ago

the working pattern was to store the plugin classes under omero/plugins and explicitly declare it in setup.py

The difference between the CLI plugins and the scripts is that in each repo, there is only 1 CLI plugin, whereas in this repo, we have quite a few scripts. Does that mean that each script must be declared as a package explicitly ? If so, I do not think this brings us any win in the sense that this PR wanted to achieve - i.e. the need to add any new subfolder when the scripts expand and evolve is still there ?

sbesson commented 1 year ago

Does that mean that each script must be declared as a package explicitly ?

No I don't think we don't need to go that far. My question is less about the individual modules/scripts and more about their organisation into packages. More generally, if you have projectA defined as follows:

setup.py
omero/
    __init__.py  # Namespace package __init__.py
    package_a/
        __init__.py  # Sub-package __init__.py
        ...

and projectB defined as follows

setup.py
omero/
    __init__.py  # Namespace package __init__.py
    package_b/
        __init__.py  # Sub-package __init__.py
        ...

is there a risk of potential issue/conflict/collision when installing deploying projectA and projectB within the same environment? If the answer is no, the main thing we probably need to watch out is that every new omero.<package_name> should remain unique across our repositories.

If so, I do not think this brings us any win in the sense that this PR wanted to achieve - i.e. the need to add any new subfolder when the scripts expand and evolve is still there ?

So far, I understand subfolders are only modified when new script categories are created (or removed), is that correct? From the history, this has only happened on only a few occasions in the last decade. Are we expecting more to happen in the future?

NB: omero/__init__.py being my primary concern here, we might be able to work around the issue by using implicit namespace packages as described in https://packaging.python.org/en/latest/guides/packaging-namespace-packages/#native-namespace-packages

pwalczysko commented 1 year ago

So far, I understand subfolders are only modified when new script categories are created (or removed), is that correct? From the history, this has only happened on only a few occasions in the last decade. Are we expecting more to happen in the future?

Yes, I think kthis is correct. Hence I am removing the contentious logic and only adding an __init__.py file on the level of the subfolder, just as it is atm for other subfolders in scripts/omero/*.

pwalczysko commented 1 year ago

The last change should simplify the problem and get us into the position where we could re-tag and release this repo to get the new scripts to the users ?