glue-viz / glue-astronomy

Plugin to add astronomy-specific functionality to glue
https://glue-astronomy.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
11 stars 12 forks source link

Make spectral-cube optional dependency #62

Open pllim opened 2 years ago

pllim commented 2 years ago

glue-astronomy pulls in spectral-cube no matter what but Jdaviz does not need it anymore. Also see:

We cannot make glue-astronomy an optional dependency but we also do not want spectral-cube to be installed unnecessarily.

dhomeier commented 1 year ago

@astrofrog do you have an idea how to conditionally enable a @data_translator? Can't think of an elegant way to do this. At the same time #54 is a request to extend spectral-cube support.

pllim commented 1 year ago

This is still a pain for us. Occasionally we get help call about some dependency from this package that broke Jdaviz installation, some CASA IO stuff that we never use.

dhomeier commented 1 year ago

FWIW the "brute force" way of putting the entire translator into a

try:
    from spectral_cube import SpectralCube

just works; would only need to update the tests to probe for spectral_cube availability (or put it into the test extras_require require instead).

And maybe putting the whole import into a wrapper file somehow like in https://github.com/glue-viz/glue/blob/f8c1e9646e7252aa13f5f58c4838cb9b670875d3/glue/core/data_factories/dendrogram.py#L6-L7 would be a more elegant solution?

pllim commented 1 year ago

This is too hacky?

Change this:

https://github.com/glue-viz/glue-astronomy/blob/bae4ecfceb3be7c2bff6043910aaa1d356f6e0a2/glue_astronomy/translators/spectral_cube.py#L9

To this:

try:
    from spectral_cube import SpectralCube, BooleanArrayMask
except ImportError:
    class SpectralCube:
        pass
    BooleanArrayMask = None
dhomeier commented 1 year ago

Definitely much shorter, looks OK to me as well. 😃 Was just wondering if some extra warning is due on the non-available translator, but since one could not even do a data.get_object(SpectralCube, ...) I guess that would sort itself out.

pllim commented 1 year ago

Jdaviz now running into problem with Python 3.11 because glue-astronomy is pulling in spectral-cube which pulls in yt which pulls in h5py that cannot build. To make things worse, yt pinned a maxversion of h5py so even if they fix it, this problem won't go away. FYI.

pllim commented 1 year ago

Okay, I would like to apologize for blaming that on spectral-cube, it is actually glue-core. I'll open a separate issue about that.