| Message ID | CAO+9K5pAJOaLoJ_c0eF3hRUO+BcLM1=AXTBq_QzAAX6zwcxN0Q@mail.gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#51765] | expand |
Hi Sharlatan, I am commenting on the second patch and will comment on the others in this same message. > + (arguments > + (list > + ;; FIXME: (Sharlatan-20211229T160902+0000): tests run via tox > + #:tests? #f)) The procedure is more or less the same for all Astropy related packages. They use tox as an abstraction for pytest. You can run tests by overriding the check phase: > (arguments > `(#:phases > (modify-phases %standard-phases > (replace 'check > (lambda* (#:key inputs outputs tests? #:allow-other-keys) > (when tests? > (add-installed-pythonpath inputs outputs) > (invoke "python" "-m" "pytest"))))))) I also had to add a few more native inputs: > (native-inputs > (list python-jsonschema > python-numpy > python-pytest > python-pyyaml > python-semantic-version > python-setuptools-scm)) It should work all the same for the remaining packages in the series. There's no need to add a comment about the copyright holder: > + ;; Copyright (C) 2021 Association of Universities for Research > in Astronomy (AURA) > + ;; > https://github.com/asdf-format/asdf-transform-schemas/blob/master/LICENSE Some of the other patches like 3 and 4 do not follow description standards like missing full sentences and full stop. You can run `guix lint` in the packages to catch some linting errors. In patch 6, gexp is being used without ungexp. In that case it shouldn't be used. `(arguments (list ...))` should be `(arguments `())` when gexp isn't being used, same for the other patches too. gwcs license is bsd-3 according to copyright headers and the LICENSE file in the license/ directory. Inputs in patches 5 and 6 may also have to be propagated if they are libraries.
Hi! Thank you for fead back @Vinicius Monego ! I've added aditional changes to all mentioned patches but I've not created patch list from scratch. Let me know if would be easy to merge patches first. Sharlatan Hellseher (5): gnu: asdf-transform-schemas: Enable tests gnu: asdf-coordinates-schemas: Enable tests gnu: asdf-astropy: Enable tests gnu: asdf-wcs-schemas: Enable tests gnu: gwcs: Refactor package gnu/packages/astronomy.scm | 101 +++++++++++++++++++++---------------- 1 file changed, 57 insertions(+), 44 deletions(-) base-commit: 4c812db049d5c9f2c438748e180f9486ad221b0a prerequisite-patch-id: 56820d9ea09a7a53f0c576466db455e706707db2 prerequisite-patch-id: ad03d8286b6789e5c16216f66766ffa403ba2e87 prerequisite-patch-id: 7ada40e42ec009357b74914aa3ed3e31bc0924aa prerequisite-patch-id: 18f6a0a2503f7359d051f10cfe01e421a3c20511 prerequisite-patch-id: dabf500576aec3742492c67f9a80fabf56872502 prerequisite-patch-id: ee0536ce1c726c3c7cea82067b7bc983d785139c On Tue, 4 Jan 2022 at 02:41, Vinicius Monego <monego@posteo.net> wrote: > > Hi Sharlatan, > > I am commenting on the second patch and will comment on the others in > this same message. > > > + (arguments > > + (list > > + ;; FIXME: (Sharlatan-20211229T160902+0000): tests run via tox > > + #:tests? #f)) > > The procedure is more or less the same for all Astropy related > packages. They use tox as an abstraction for pytest. You can run tests > by overriding the check phase: > > > (arguments > > `(#:phases > > (modify-phases %standard-phases > > (replace 'check > > (lambda* (#:key inputs outputs tests? #:allow-other-keys) > > (when tests? > > (add-installed-pythonpath inputs outputs) > > (invoke "python" "-m" "pytest"))))))) > > I also had to add a few more native inputs: > > > (native-inputs > > (list python-jsonschema > > python-numpy > > python-pytest > > python-pyyaml > > python-semantic-version > > python-setuptools-scm)) > > It should work all the same for the remaining packages in the series. > > There's no need to add a comment about the copyright holder: > > > + ;; Copyright (C) 2021 Association of Universities for Research > > in Astronomy (AURA) > > + ;; > > https://github.com/asdf-format/asdf-transform-schemas/blob/master/LICENSE > > Some of the other patches like 3 and 4 do not follow description > standards like missing full sentences and full stop. You can run `guix > lint` in the packages to catch some linting errors. > > In patch 6, gexp is being used without ungexp. In that case it > shouldn't be used. `(arguments (list ...))` should be `(arguments `())` > when gexp isn't being used, same for the other patches too. gwcs > license is bsd-3 according to copyright headers and the LICENSE file in > the license/ directory. Inputs in patches 5 and 6 may also have to be > propagated if they are libraries. >
Em ter, 2022-01-04 às 21:39 +0000, Sharlatan Hellseher escreveu: > Hi! > > Thank you for fead back @Vinicius Monego ! > > I've added aditional changes to all mentioned patches but I've not > created patch list from scratch. Let me know if would be easy to > merge > patches first. > The best way is to rebase to and amend the commits in the series and send it from scratch again formatted with v2, v3, etc using --reroll- count=2 in git format-patch. You can also submit the patches using `git send-email -- to="NNN@debbugs.gnu.org" <patches>` where NNN is the ID of the issue, here 51765. Check [1] to see how to configure it. If you can merge them somehow it is preferable. [1] https://git-send-email.io/
Hi Guix team! I've setup git send-email (finally!) but strugled with rebase and amend my old commit into new ones. I've pul lates changes from upstream add combined changes per commit: Sharlatan Hellseher (6): gnu: python-asdf: Update to 2.8.3 gnu: Add asdf-transform-schemas gnu: Add asdf-coordinates-schemas gnu: Add asdf-astropy gnu: Add asdf-wcs-schemas gnu: Add gwcs gnu/packages/astronomy.scm | 222 ++++++++++++++++++++++++++++++++++--- 1 file changed, 204 insertions(+), 18 deletions(-) base-commit: 056935506b8b5550ebeb3acfc1d0c3b4f11b6a2e On Tue, 4 Jan 2022 at 23:01, Vinicius Monego <monego@posteo.net> wrote: > > Em ter, 2022-01-04 às 21:39 +0000, Sharlatan Hellseher escreveu: > > Hi! > > > > Thank you for fead back @Vinicius Monego ! > > > > I've added aditional changes to all mentioned patches but I've not > > created patch list from scratch. Let me know if would be easy to > > merge > > patches first. > > > > The best way is to rebase to and amend the commits in the series and > send it from scratch again formatted with v2, v3, etc using --reroll- > count=2 in git format-patch. > > You can also submit the patches using `git send-email -- > to="NNN@debbugs.gnu.org" <patches>` where NNN is the ID of the issue, > here 51765. Check [1] to see how to configure it. > > If you can merge them somehow it is preferable. > > [1] https://git-send-email.io/ > >
From b1ec99ee7222751e7b7393b3ea2ce32e6131a0d3 Mon Sep 17 00:00:00 2001 From: Sharlatan Hellseher <sharlatanus@gmail.com> Date: Sun, 2 Jan 2022 21:10:12 +0000 Subject: [PATCH 2/6] gnu: Add asdf-transform-schemas * gnu/packages/astronomy.scm: (python-asdf-transform-schemas): New variable. --- gnu/packages/astronomy.scm | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm index 5b5887fb2e..8d08c5421d 100644 --- a/gnu/packages/astronomy.scm +++ b/gnu/packages/astronomy.scm @@ -1268,6 +1268,37 @@ (define-public python-asdf implementation of the ASDF Standard.") (license license:bsd-3))) +(define-public python-asdf-transform-schemas + (package + (name "python-asdf-transform-schemas") + (version "0.2.0") + (source + (origin + (method url-fetch) + (uri (pypi-uri "asdf_transform_schemas" version)) + (sha256 + (base32 "1gmzd81hw4ppsvzrc91wcbjpcw9hhv9gavllv7nyi7qjb54c837g")))) + (build-system python-build-system) + (arguments + (list + ;; FIXME: (Sharlatan-20211229T160902+0000): tests run via tox + #:tests? #f)) + (native-inputs + (list python-semantic-version + python-setuptools-scm)) + (propagated-inputs + (list python-asdf)) + (home-page "https://github.com/asdf-format/asdf-transform-schemas") + (synopsis "ASDF schemas for transforms") + (description + "This package provides ASDF schemas for validating transform tags. Users +should not need to install this directly; instead, install an implementation +package such as asdf-astropy, which includes asdf-transform-schemas as a +dependency.") + ;; Copyright (C) 2021 Association of Universities for Research in Astronomy (AURA) + ;; https://github.com/asdf-format/asdf-transform-schemas/blob/master/LICENSE + (license license:bsd-3))) + (define-public python-astroalign (package (name "python-astroalign") -- 2.34.0