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

Make spectral-cube optional dependency #62

Open
pllim opened this issue Feb 1, 2022 · 7 comments
Open

Make spectral-cube optional dependency #62

pllim opened this issue Feb 1, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@pllim
Copy link
Contributor

pllim commented Feb 1, 2022

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.

@pllim pllim added the enhancement New feature or request label Feb 1, 2022
@dhomeier
Copy link
Contributor

dhomeier commented Aug 16, 2022

@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
Copy link
Contributor Author

pllim commented Aug 16, 2022

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
Copy link
Contributor

dhomeier commented Aug 16, 2022

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
Copy link
Contributor Author

pllim commented Aug 16, 2022

This is too hacky?

Change this:

from spectral_cube import SpectralCube, BooleanArrayMask

To this:

try:
    from spectral_cube import SpectralCube, BooleanArrayMask
except ImportError:
    class SpectralCube:
        pass
    BooleanArrayMask = None

@dhomeier
Copy link
Contributor

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

This comment was marked as outdated.

@pllim
Copy link
Contributor Author

pllim commented Oct 24, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants