-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Open
Description
Description
As astropy grows over the years, we now have different kinds of dependencies:
- Absolutely required and documented as such:
numpy - Required for certain jobs, e.g., building docs and testing.
- Declared optional but really required for some subpackages, such as but not limited to:
matplotlibforvisualization.wcsaxesasdfforio.misc.asdf(this one is even cyclic, i.e., we also depend on it for schemas version or something)PyYAMLforio.misc.yamland maybe evenio.fits(see Table.read on FITS file triggers PyYAML import #7783)
- Truly optional, i.e., there is a fallback alternative if the dependency is missing -- Do we really have this? Need to double check.
All the dependencies are declared in the installation page but unfortunately this page is purely informational and does not enforce anything code-wise; i.e., it will not automatically downloads asdf for you if you use astropy.io.misc.asdf. (But do we even want that?)
The complexity of these dependencies occasionally cause problems when they are missing or incompatible, resulting in but not limited to:
- Unable to use part of the code directly.
- Unable to use code in downstream package that uses our affected code.
- Unable to test the code effectively, causing bugs to slip through during development process. (Either forget to test with or without the dependency.)
- Version limitations for optional dependencies are ignored #12121
Some workaround to make the dependency optional also caused performance issue; e.g., #10302 .
Things we could do to improve the situation
- Import-only test job as discussed in Add an import-only test matrix entry #7939 . Such a job will ensure things like, but not limited to:
pytestnot required at runtime.- Optional dependencies don't cross-pollinate; e.g.,
io.fitswould work even ifPyYAMLis not present, orvisualization.wcsaxesis not affected by the presence or lack ofasdf. - Importing
astropywithout any optional dependencies will not causeImportError. - Defining special cases where
ImportErroris acceptable; e.g.,io.misc.asdfcan throwImportErrorifasdfis not installed.- However, will this accidentally let real problems through?
- Can we overload
__doctest_requires__for this purpose or is subpackageconftest.pyincantation sufficient? - Perhaps even a new
__doctest_subpackage_requires__directive inpytest-doctestplusto avoid recursion into a subpackage.
- Skip test collection in
setup_package.pyto avoid problem with requiringextension-helpers; e.g. Tests should collect without optional dependencies #10246 . - Do not use
pytest.importorskipin non-test code; e.g., Speed up wcsaxes import by avoiding unnecessary pytest import #10302 vs TST: Globally ignore import error during test collection for doctest #10325. - Avoid
try ... except ...imports in subpackage; e.g.,io.misc.asdfshould always assumeasdfis importable. - Parse
__init__.pywith AST rather than importing them? (This idea was given with a 😉)
What else?
Other related issues/PRs not mentioned above
- MNT: Make asdf import truly optional in io.misc.asdf #10260 (deemed impractical and was rejected)
- Why is this not part of pytest? scientific-python/pytest-doctestplus#100
- add
doctest_ignore_import_errors_modulesscientific-python/pytest-doctestplus#110
Disclaimer
The information here is put together from multiple discussions with many people. Feel free to comment/edit if I missed something.
Reactions are currently unavailable