unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Sharlatan Hellseher <sharlatanus@gmail.com>
To: Vinicius Monego <monego@posteo.net>
Cc: 48046@debbugs.gnu.org
Subject: [bug#48046] [PATCH]: Gnu add astropy
Date: Sun, 23 May 2021 20:01:00 +0000	[thread overview]
Message-ID: <CAO+9K5oMqiq5JUtyhNhBvBUDs5bmfguRrcT4cWOkNz4wGLgYrw@mail.gmail.com> (raw)
In-Reply-To: <7055d87cdde449415e62f0b0c15d96deb378af67.camel@posteo.net>

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



-- 

… наш разум - превосходная объяснительная машина которая способна
найти смысл почти в чем угодно, истолковать любой феномен, но
совершенно не в состоянии принять мысль о непредсказуемости.




  reply	other threads:[~2021-05-23 20:02 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-26 20:01 [bug#48046] [PATCH]: Gnu add astropy Sharlatan Hellseher
2021-05-19 18:16 ` Vinicius Monego
2021-05-22 20:00   ` Sharlatan Hellseher
2021-05-23 17:54     ` Vinicius Monego
2021-05-23 20:01       ` Sharlatan Hellseher [this message]
2021-10-29 21:35         ` Sharlatan Hellseher
2021-10-29 22:58           ` Vinicius Monego
2021-10-30  2:51 ` [bug#48046] [PATCH v2 0/3] Add Astropy Vinicius Monego
2021-10-30  2:51   ` [bug#48046] [PATCH v2 1/3] gnu: python-pytest-astropy: Adjust inputs Vinicius Monego
2021-10-30  2:51   ` [bug#48046] [PATCH v2 2/3] gnu: python-pyerfa: " Vinicius Monego
2021-10-30  2:51   ` [bug#48046] [PATCH v2 3/3] gnu: Add python-astropy Vinicius Monego
2021-11-08  8:06   ` bug#48046: [PATCH v2 0/3] Add Astropy Efraim Flashner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAO+9K5oMqiq5JUtyhNhBvBUDs5bmfguRrcT4cWOkNz4wGLgYrw@mail.gmail.com \
    --to=sharlatanus@gmail.com \
    --cc=48046@debbugs.gnu.org \
    --cc=monego@posteo.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).