Subject: Re: [PATCH]: Gnu add astropy

From: Vinicius Monego <[email-masked]>
Date: Fri, 29 Oct 2021 22:58:45 +0000

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
> <[email-masked]> 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 <[email-masked]>
> > 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
> > 
> > 
> > 
> > --
> > 
> > … наш разум - превосходная объяснительная машина которая способна
> > найти смысл почти в чем угодно, истолковать любой феномен, но
> > совершенно не в состоянии принять мысль о непредсказуемости.
> 
> 
>