guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#48046] [PATCH]: Gnu add astropy


From: Vinicius Monego
Subject: [bug#48046] [PATCH]: Gnu add astropy
Date: Sun, 23 May 2021 17:54:08 +0000

Hi Sharlatan,

Thanks for continuing the work on the astropy package, I managed to
finish it this time. I am resending your patch with the following
modifications:

- Moved the package definition from the bottom to the middle of the
file (to avoid merge conflicts)
- Removed all optional inputs and propagated the remaining. I left only
those listed in install_requires, setup_requires, test_requires and
test[extras] in setup.cfg
- Changed synopsis and description
- Changed package labels to match the package name    
- Made the compiler file writable instead of deleting it
- Deleted the makdir-astropy phase (it wasn't needed)
- Added license for the jquery bundle that is not replaced

and then I made my own improvements on that patch: enabling tests and
unbundling some external libraries.

I removed the optional packages because astropy is a core package,
which will be a dependency for its many extensions. It's important that
it builds with a high probability of success or the chain will break.
Some of its optional dependencies, e.g. Pandas, have a broken build in
aarch64 at the moment. The "full" astropy package could be installed
easily from a manifest file and the tests can run again with
astropy.test().

> the project heavily depends on TOX which requires pip to install
> missing dependencies for itself.

I don't think that a project can heavily depend on tox, all it does is
manage a virtual environment with dependencies to run the tests. Guix
does the same so tox is redundant here. Tests will still run with the
testing framework.

Two more suggestions for future Python patches:

> +         (replace 'check
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (add-installed-pythonpath inputs outputs)
> +             (invoke "pytest" "-vv")))

When a project contains tests as part of the application code, as in
Astropy, tests should run with "pytest --pyargs module". See Pytest
Integration Pratices:
https://docs.pytest.org/en/documentation-restructure/background/goodpractices.html

It's also good practice in Guix to use (when tests?) when overriding
the check phase to allow --without-tests=pkg.
  
> ImportError: You appear to be trying to import astropy from within a
> source checkout or from an editable installation without building the
> extension modules first. Either run:

I fixed this error by running the second command before the tests.

If you don't mind the modifications I did, I will call this patchset
complete and wait for a committer to review.

Vinicius

Attachment: 0001-gnu-Add-python-astropy.patch
Description: Text Data

Attachment: 0002-gnu-python-pytest-astropy-Propagate-inputs.patch
Description: Text Data

Attachment: 0003-gnu-python-pyerfa-Adjust-inputs.patch
Description: Text Data

Attachment: 0004-gnu-Add-wcslib-7.3.patch
Description: Text Data

Attachment: 0005-gnu-python-astropy-Enable-tests.patch
Description: Text Data

Attachment: 0006-gnu-python-astropy-Unbundle-ply-and-configobj.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]