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

Not all dependencies are included in the setup.py #18

Open
bhazelton opened this issue Oct 27, 2021 · 8 comments
Open

Not all dependencies are included in the setup.py #18

bhazelton opened this issue Oct 27, 2021 · 8 comments
Assignees

Comments

@bhazelton
Copy link
Member

One of our undergrads stumbled on this. In particular bokeh is not listed in the setup.py but is imported at the top of utils.py. I haven't checked if there are other requirements that are not listed.

A side note -- the imports in that file are difficult to read. There are recommended structures for imports, see e.g. https://www.python.org/dev/peps/pep-0008/#imports

@jsdillon
Copy link
Member

@HonggeunKim I think that was you. Can you take care of this?

@HonggeunKim
Copy link
Contributor

@jsdillon Yes I think it is me. bokeh is a python package to make an interactive plot for delay spectra. Do I need to add bokeh in 'install_requires' of setup.py or should I do something else?

@PyxieLouStar
Copy link

We're also getting an import error for healpy. Running from hera_notebook_templates import utils gives the print message could not import sklearn, but does not throw an error so it isn't clear if the sklearn package is actually required.

@bhazelton
Copy link
Member Author

We should avoid using healpy, it's licensed under the GPL license, which is a copy left license and is not compatible with our licensing policies. The astropy_healpix package should be used instead.

@HonggeunKim
Copy link
Contributor

@jsdillon I added 'bokeh' and 'multiprocessing' to the 'linstall_requires' list of setup.py file and directly committed it to the master branch

@jsdillon
Copy link
Member

ok, thanks!

@bhazelton
Copy link
Member Author

I'm not sure this should be closed. Healpy is still not listed in the setup.py but is imported at the top of utils.py. I also don't think we should be importing it under the license of this repo.

@jsdillon
Copy link
Member

@dstorer Looks like its just your code that uses healpy. How hard would it be to change it to astropy-healpix?

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

No branches or pull requests

5 participants