Subject: Re: Rebase it for the new python packages

From: Vinicius Monego <[email-masked]>
Date: Thu, 03 Feb 2022 20:49:08 +0000

Em qui, 2022-02-03 às 20:29 +0100, Vivien escreveu:
> Dear guix,
> 
> We have 2 new python packages, so my patch series creates a trivial
> conflict. Here is a new version that you can apply directly.
> 
> Best regards,
> 
> Vivien

Hi,

To avoid future merge conflicts, please move the packages somewhere in
the middle of the files instead of the bottom.

As a rule of thumb for Python packages with tests in Pytest, the check
phase is overriden and Pytest is called manually. When the tests are in
a subfolder inside the module, add a --pyargs <package> parameter to
the pytest command, see e.g. the python-cartopy package. I could run
the python-nibabel tests with this change without having to delete
anything.

If the tests still can't run because of missing data, it's fine to
source from the upstream repository instead of PyPI, or skip the few
tests that need them or at all if the repository doesn't ship a
setup.py. If tests are to be disabled, they should also have a comment
with the reason.

The 'test-less' packages shouldn't be needed AFAICS. Tests should run
by overriding the check phase as stated above (untested).

I also have a few comments about the patches in general:

> +    (source (origin
> +              (method url-fetch)
> +              (uri (pypi-uri "imageio-ffmpeg" version))
> +              (sha256
> +               (base32
> +               
> "0ff14079izsyxwf6ki68k9a7w5krjlal7lwqvzg2bbddl92l5spj"))))

Could you style it as

    (source
     (origin
       (method url-fetch)
       (uri (pypi-uri "imageio-ffmpeg" version))
       (sha256
        (base32
"0ff14079izsyxwf6ki68k9a7w5krjlal7lwqvzg2bbddl92l5spj"))))

and the other packages too?

Gexps should only be used when ungexp (#$) is used. On many patches
(e.g. python-nitime) ungexp is not being used.

When using gexp, it's better to style the arguments as:

+    (arguments
+     (list
+       #:phases
+       #~(modify-phases %standard-phases

to save columns (some of the packages exceeded the 78 columns limit),
instead of

> +    (arguments
> +     (list #:phases
> +           #~(modify-phases %standard-phases
> 

.

Some of the descriptions are not full sentences (e.g. in python-pytest-
harvest-minimal). Please check that descriptions are full sentences.

When sending an updated series, use patch versions with --reroll-
count=4 or -v4.

Could you send a v4 with the requested changes?

Vinicius