[bug#48046] : Gnu add astropy

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

Hi Vinicius,

It' fantastic! Thanks for your feedback and modification, I'm
absolutely ok with them.

When astropy is accepted it will open a way for other astronomical
packages which are depend on it,
and I've noticed some other packages in master require astropy for
tests so it would beneficial to have it merged :)!

On Sun, 23 May 2021 at 17:54, Vinicius Monego <monego@posteo.net> wrote:
>
> 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

Hi Guix team!

Is this set of patches still in review or just dromaint :)?

Regards

On Sun, 23 May 2021 at 21:01, Sharlatan Hellseher <sharlatanus@gmail.com> wrote:
>
> Hi Vinicius,
>
> It' fantastic! Thanks for your feedback and modification, I'm
> absolutely ok with them.
>
> When astropy is accepted it will open a way for other astronomical
> packages which are depend on it,
> and I've noticed some other packages in master require astropy for
> tests so it would beneficial to have it merged :)!
>
> On Sun, 23 May 2021 at 17:54, Vinicius Monego <monego@posteo.net> wrote:
> >
> > 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
>
>
>
> --
>
> … наш разум - превосходная объяснительная машина которая способна
> найти смысл почти в чем угодно, истолковать любой феномен, но
> совершенно не в состоянии принять мысль о непредсказуемости.

Hi Sharlatan,

I was looking at this series once again and there are some fixes I will
have to do in a resubmission.

The main problem right now is that astropy is tied to a specific wcs
version that it bundles. The code can't compile with the 7.5 version in
Guix. Astropy 4.2 comes with wcs 7.3 and 4.3 comes with wcs 7.6. I
tried to update wcs in guix to 7.6 but tests failed in this version.
Not sure what is the best approach in this, packaging a separate wcs at
7.3 and keep astropy at 4.2.1, or use the bundle.

Anyway, I'll submit this series again later.

Vinicius

Em sex, 2021-10-29 às 22:35 +0100, Sharlatan Hellseher escreveu:
> Hi Guix team!
> 
> Is this set of patches still in review or just dromaint :)?
> 
> Regards
> 
> On Sun, 23 May 2021 at 21:01, Sharlatan Hellseher
> <sharlatanus@gmail.com> wrote:
> > 
> > Hi Vinicius,
> > 
> > It' fantastic! Thanks for your feedback and modification, I'm
> > absolutely ok with them.
> > 
> > When astropy is accepted it will open a way for other astronomical
> > packages which are depend on it,
> > and I've noticed some other packages in master require astropy for
> > tests so it would beneficial to have it merged :)!
> > 
> > On Sun, 23 May 2021 at 17:54, Vinicius Monego <monego@posteo.net>
> > wrote:
> > > 
> > > 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
> > 
> > 
> > 
> > --
> > 
> > … наш разум - превосходная объяснительная машина которая способна
> > найти смысл почти в чем угодно, истолковать любой феномен, но
> > совершенно не в состоянии принять мысль о непредсказуемости.
> 
> 
>