cta-observatory / ctapipe

Low-level data processing pipeline software for CTAO or similar arrays of Imaging Atmospheric Cherenkov Telescopes
https://ctapipe.readthedocs.org
BSD 3-Clause "New" or "Revised" License
64 stars 268 forks source link

Improve info tool dependencies option #2303

Open HealthyPear opened 1 year ago

HealthyPear commented 1 year ago

While porting this for SWGO software I modified the code to read the dependencies at runtime from setup.cfg (like we do already for the Sphinx documentation configuration).

While doing this I noticed that the code using importlib.import_module sometimes gives rise to a ModuleNotFoundErrorexception, so I added it to the first try/except block to fallback to importlib.metadata.version and only if that also fails we give up.

I also used pytest-console-scripts to create 1 test for each flag (but see this issue I opened there, even though I tested each of them separately).

HealthyPear commented 1 year ago

It seems that the online tests fail to find the option part of setup.cfg or the file itself.

Locally it works as expected.

maxnoe commented 1 year ago

The tests are run on the installed ctapipe, which doesn't have a setup.cfg

HealthyPear commented 1 year ago

The tests are run on the installed ctapipe, which doesn't have a setup.cfg

Dammit...

And I guess this https://github.com/pypa/packaging-problems/issues/215 kills the possibility to use e.g importlib.metadata to access this information from user-like installations, am I right?

Can't we run the tests on the development version and in a separate job just test that the user installation simply doesn't crash? The code to test is the same.

HealthyPear commented 1 year ago

It seems that there is at least importlib.metadata.requires

kosack commented 1 year ago

You can't read setup.cfg in ctapipe-info, since that file is outside of the package. But perhaps there is a way to do something similar to how we do the version string: there a file gets generated during the install process that becomes part of the package. So in the install, you could generate a "dependencies" list and then that would be available in the package.

The main issue I have with that is that ctapipe-info should also list optional dependencies, since that info is very useful to know for debugging. But I suppose you could parse options.extras_require, but then you might get lots of random things about the documentation building, and not generally optional run-time packages like bokeh (which now is required, but should be optional)

maxnoe commented 1 year ago

In general, any functionality like this should use the new importlib.metadata (importlib_metadata before 3.9) module prodiving such information independently of the setup.cfg / pyproject.toml or whatever packaging system is used.

HealthyPear commented 1 year ago

I used importlib.metadata to extract the information about the complete set of packages and I added the splitting of optional packages based on the extras provided by the package metadata.

The internal approach is somewhat more brute-force than I wished: it appears that from Python3.10 that library allows using JSON to explore the metadata and simplifies it a bit, but since we test from 3.9 I had to keep things compatible with the older API.

The unit-tests I added based on pytest-console-scripts now work as intended for some reason.

HealthyPear commented 1 year ago

On a side note: shouldn't ctapipe-info --resources also show the existing cached resources (i.e. ~/.cache/ctapipe/....)?

kosack commented 1 year ago

On a side note: shouldn't ctapipe-info --resources also show the existing cached resources (i.e. ~/.cache/ctapipe/....)?

It does:

> ctapipe-info --resources  
*** ctapipe resources ***

CTAPIPE_SVC_PATH: (directories where resources are searched)
     no path is set

RESOURCE NAME                  : LOCATION
----------------------------------------------------------------------
ASTRICam.camgeom.fits.gz       : ~/.cache/ctapipe/cccta-dataser
CHEC.camgeom.fits.gz           : ~/.cache/ctapipe/cccta-dataser
DigiCam.camgeom.fits.gz        : ~/.cache/ctapipe/cccta-dataser
FACT.camgeom.fits.gz           : ~/.cache/ctapipe/cccta-dataser