sczesla / PyAstronomy

A collection of astronomy-related routines in Python
152 stars 35 forks source link

Install dependencies with pypi package #52

Closed HarHarLinks closed 2 years ago

HarHarLinks commented 3 years ago

I want to use this as a library, so I pip install --upgrade PyAstronomy it into my virtual environment (using pyenv). Yet, when running my program:

[...]
    from PyAstronomy import pyasl
  File "~/.pyenv/versions/openweathermap/lib/python3.9/site-packages/PyAstronomy/pyaC/__init__.py", line 3, in <module>
    _ic = ImportCheck(["numpy", "scipy", "ssl"], required=["numpy", "scipy"])
  File "~/.pyenv/versions/openweathermap/lib/python3.9/site-packages/PyAstronomy/pyaC/importCheck.py", line 105, in __init__
    raise(PE.PyARequiredImport(
PyAstronomy.pyaC.pyaErrors.pyaOtherErrors.PyARequiredImport: 
---------------------
A PyA error occurred:
---------------------
Type of error: PyA import error
What happened?
    Could not import required module(s): scipy
What are possible solutions?
  - Please install scipy

So I checked it out:

$ pip show pyastronomy
Name: PyAstronomy
Version: 0.16.0
Summary: A collection of astronomy related tools for Python.
Home-page: https://github.com/sczesla/PyAstronomy
Author: PyA group
Author-email: stefan.czesla@hs.uni-hamburg.de
License: MIT Licence
Location: ~/.pyenv/versions/openweathermap/lib/python3.9/site-packages
Requires: six, numpy
Required-by:

Apparently pip does not know about the scipy dependency. Further it also is not a recursive dependency:

$ pipdeptree -p PyAstronomy
PyAstronomy==0.16.0
  - numpy [required: Any, installed: 1.21.0]
  - six [required: Any, installed: 1.16.0]

Yet, your requirements.txt lists it (and much more) as dependencies.

What is going on? If scipy is a hard dependency, why isn't it automatically installed?

DanielAndreasen commented 3 years ago

I guess it comes from here: https://github.com/sczesla/PyAstronomy/blob/master/setup.py#L207

HarHarLinks commented 3 years ago

I don't know much about packaging. Would it be a good idea to read the requirements.txt into that list instead?

DanielAndreasen commented 3 years ago

I have heard worse ideas. But packaging is tricky in my experience. It could be that a simple solution like that would work.

sczesla commented 3 years ago

Hello, the origin of the problem is inconsistencies in the dependency management of PyAstronomy. What you see is a long-range ramification of an initial intention to keep the number of required dependencies as small as possible. Not every part of PyA require scipy (but many) and scipy is a pretty heavy-weight package. Such "optional dependencies" must be managed and that is error prone (on the plus side, at least it tells you what to do and the solution is fortunately simple).

Daniel is right in pointing out the install_ requires argument from setup.py. The dependencies listed there are installed along with PyA on a call to "pip install PyAstronomy", the dependencies in requirements are not. It is generally considered good style (as far as I know) to provide a minimal list of dependencies in install_requirements and a more exhaustive recipe for an environment in requirements.txt. My impression is that this list is simply too greedy. Scipy is so often required that it should go in along with some others. I think on most systems using PyA, scipy is typically installed already.

HarHarLinks commented 3 years ago

Perhaps PyA can provide flavors for certain features such as pip install PyAstronomy[full], pip install PyAstronomy[asl] that also installs all optional/respective dependencies? And, if an import fails like in my case, it would be nice to then suggest installing that flavor such that the dependency tree is still correct. https://creatronix.de/pip-optional-dependencies/

sczesla commented 3 years ago

This is another possibility. One such option already exists, which is used to prevent some FORTRAN dependencies to break the installation. In my opinion, more of these would not help the clarity of the installation process. I looked over the real dependencies in the code and added scipy plus some minor libraries to the install_requires list. My feeling is that this would be installed anywhere in most environments PyA would be working in. Installation attempt and import check worked so far (in pristine environment). As far as I could see, matplotlib is not causing similar trouble so far, but there is a risk...

sczesla commented 2 years ago

OK. A new version with some more comprehensive install requirements is out. Hope that makes life easier. Cheers