usnistgov / imppy3d

Image processing in python for 3d image stacks
MIT License
15 stars 1 forks source link

[JOSS] Python packages should be `pip install`able #3

Open sitic opened 2 weeks ago

sitic commented 2 weeks ago

The JOSS review guidelines state:

[...] For example, Python packages should be pip installable, and have adopted packaging conventions, while Fortran submissions with a Makefile may be sufficient.

Good: The software is simple to install, and follows established distribution and dependency management approaches for the language being used
OK: A list of dependencies to install, together with some kind of script to handle their installation (e.g., a Makefile)
Bad (not acceptable): Dependencies are unclear, and/or installation process lacks automation

I think IMPPY3D should be a package that can be imported and should also be pip installable. That way it can easily be used in other projects and by all users (e.g. on MacOS, with python versions where the Cython code hasn't been precompiled etc). As anyone with a basic knowledge of python should be able to use IMPPY3D it needs to be very easily installable.

The packaging isn't too complex and shouldn't interfere with the existing Conda/Mamba way too much.

I've created some basic scaffolding to show in principle how to the python packaging. See the PR for details.

A release should then also be uploaded to PyPi, then pip install imppy3d should be able to do all the heavy lifting.

Linkback: https://github.com/openjournals/joss-reviews/issues/7405

gknapp1 commented 1 week ago

I haven't tested all of the functionality yet, but a local pip install (pip install . from the repo's root directory) does work on the main branch with a minor change. To get working, all version specifiers stating a version "equal to" in setup.py need to use == instead of =.

gknapp1 commented 2 days ago

@NM0ser, thanks for getting this up on PyPi. I was able to install using Python 3.10.11.

However, to run the examples (specifically the "calc_metrics_pore" example) I needed to additionally pip install the imagecodecs package. Once I did that, all the examples ran as expected. I see that imagecodecs is commented out in setup.py. Is there a reason for not including that in the build?

NM0ser commented 1 day ago

Thanks so much for the testing, @gknapp1! PyPi has been a learning process, but it looks like it is coming together now. Regarding imagecodecs, I had to do some digging to figure out what is going on there. I commented out some library requirements in the setup.py file because I wanted to clean things up. I had libraries marked as required dependencies that I do not actually make calls to, though, they are indirectly required because I do, for example, call the scikit-image library which depends on some of those libraries. However, I should not of course require those dependencies then, scikit-image should require those dependencies.

What you found seems to be an interesting exception. As it turns out, scikit-image wraps other image libraries for importing/exporting, and the user (me) can ask to make special use of these other libraries during a function call. So, in order to make it possible to import/export multi-page TIFF files, I specified that plugin=tifffile in one of their importers/exporters. Well, as it turns out, the tifffile library has optional dependencies if you want extended features. Specifically, for compressing images, you must have imagecodecs installed -- however, it is not specified as required by the package as far as I can tell.

Long story short, I do not make any calls directly to imagecodecs, but it seems to have sneaked through the cracks. I have included it back into the setup.py file. What made this even more difficult to catch on my end is that I apparently have imagecodecs cached in my python development environment. So, even after creating a new Python environment, this somehow did not pop up on my end.

Thanks again! The latest version fixes this requirement in setup.py.