GNU bug report logs - #53395
Fix pypi import for flake8-array-spacing

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 53395 in the body.
You can then email your comments to 53395 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Vivien Kraus <vivien <at> planete-kraus.eu>
To: guix-patches <at> gnu.org
Subject: Fix pypi import for flake8-array-spacing
Date: Thu, 20 Jan 2022 20:45:19 +0100

[Message part 1 (text/plain, inline)]

Dear guix,

flake8-array-spacing is a funny package because its download URI uses a
base name of flake8_array_spacing. Apparently some other packages have
similar features, appearing as the downcase version.

What do you think?

Best regards,

Vivien

[0001-pypi-importer-Convert-to-_-in-pypi-urls-if-needed.patch (text/x-patch, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #8 received at 53395 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Vivien Kraus <vivien <at> planete-kraus.eu>
Cc: 53395 <at> debbugs.gnu.org
Subject: Re: bug#53395: Fix pypi import for flake8-array-spacing
Date: Mon, 24 Jan 2022 15:05:47 +0100

Hello!

Vivien Kraus <vivien <at> planete-kraus.eu> skribis:

> From d8923c394fbe2e8eedf6fa548455d398f0caa022 Mon Sep 17 00:00:00 2001
> From: Vivien Kraus <vivien <at> planete-kraus.eu>
> Date: Thu, 20 Jan 2022 20:11:56 +0100
> Subject: [PATCH] pypi importer: Convert - to _ in pypi urls if needed.
>
> * guix/import/pypi.scm (find-project-url): New function.
> (make-pypi-sexp): Use find-project-url.

[...]

> +(define (find-project-url name pypi-url)
> +  "Try different project name substitution until the result is found in
> +pypi-url.  Downcase is required for \"Deprecated\" and \"uWSGI\", and
> +underscores are required for flake8-array-spacing."
> +  (or (find (cut string-contains pypi-url <>)
> +            (list name
> +                  (string-downcase name)
> +                  (string-replace-substring name "-" "_")))
> +      (begin
> +        (warning (G_ "The project name `~a' does not appear in the pypi URL; you might need to fix the pypi-url declaration in the generated package.  The URL is: ~a~%")
> +                 name pypi-url)
> +        name)))

As a rule of thumb, warnings are one-line messages (not sentences)
describing the problem.  Here you could emit such a warning, followed by
a call to ‘display-hint’, which accepts Texinfo markup.

Could you adjust accordingly?

Also, what about adding a unit test?

Thanks,
Ludo’.

Message #11 received at 53395 <at> debbugs.gnu.org (full text, mbox):

From: Vivien Kraus <vivien <at> planete-kraus.eu>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 53395 <at> debbugs.gnu.org
Subject: Re: bug#53395: Fix pypi import for flake8-array-spacing
Date: Mon, 24 Jan 2022 23:07:46 +0100

[Message part 1 (text/plain, inline)]

Hi :)

Ludovic Courtès <ludo <at> gnu.org> writes:
> As a rule of thumb, warnings are one-line messages (not sentences)
> describing the problem.  Here you could emit such a warning, followed by
> a call to ‘display-hint’, which accepts Texinfo markup.
display-hint seems to always add \n\n at the end of the message, is it
intended that way? Should I do one big display-hint instead of 3?

> Also, what about adding a unit test?
I added two by copying the "foo" example and pretending it’s named "Foo"
and "goo" respectively (fixing the package name in the json data), while
keeping release URLs as /foo-… so the importer should in the first case
recognize "Foo" in the URL and in the second case it should not
recognize "goo" in the URL so it should emit a warning. I didn’t test
the dash-to-underscore transformation, because it would require test
data with a package named "foo-something" but with release URIs
containing "foo_something". That data point does not exist currently in
the test module.

Vivien

[0001-pypi-importer-Convert-to-_-in-pypi-urls-if-needed.patch (text/x-patch, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 53395 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Vivien Kraus <vivien <at> planete-kraus.eu>
Cc: 53395 <at> debbugs.gnu.org
Subject: Re: bug#53395: Fix pypi import for flake8-array-spacing
Date: Tue, 25 Jan 2022 14:19:05 +0100

Hi Vivien,

Vivien Kraus <vivien <at> planete-kraus.eu> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>> As a rule of thumb, warnings are one-line messages (not sentences)
>> describing the problem.  Here you could emit such a warning, followed by
>> a call to ‘display-hint’, which accepts Texinfo markup.
> display-hint seems to always add \n\n at the end of the message, is it
> intended that way? Should I do one big display-hint instead of 3?

Yes, just one ‘display-hint’ call, using complete sentences and
paragraphs.  The text should provide an explanation and more importantly
a hint as to what can done, like “The generated package definition might
be wrong; please double-check …”.

You can avoid @strong though, because it doesn’t do
anything useful.

> +        (warning
> +         (G_ "The project name does not appear verbatim in the pypi URI~%"))

I’d make it:

  "project name ~a does not appear verbatim in the PyPI URI~%"

>> Also, what about adding a unit test?

[...]

> +(test-equal "If the package goo is released as foo, the importer warns"
> +  "warning: The project name does not appear verbatim in the pypi URI
> +hint: The project name is: *goo*
> +
> +hint: The pypi URI is: *`https://example.com/foo-1.0.0.tar.gz'*
> +
> +hint: The pypi-uri declaration in the generated package might need to be changed.
> +
> +"
> +  (call-with-output-string
> +    (lambda (port)
> +      (parameterize ((guix-warning-port port)
> +                     (current-error-port port))
> +        ;; Replace network resources with sample data.
> +        (mock ((guix import utils) url-fetch
> +               (lambda (url file-name)
> +                 (match url

These two tests are really integration tests: they exercise the whole
shebang.  I was thinking about having a unit test of just
‘find-project-url’, which is less work (and maintenance :-)) and may be
good enough.

Perhaps you can also change one of the existing python-foo tests to
exercise this bit, for instance by making it “Foo”.

WDYT?

Thanks,
Ludo’.

Message #17 received at 53395 <at> debbugs.gnu.org (full text, mbox):

From: Vivien Kraus <vivien <at> planete-kraus.eu>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 53395 <at> debbugs.gnu.org
Subject: Re: bug#53395: Fix pypi import for flake8-array-spacing
Date: Tue, 25 Jan 2022 17:39:54 +0100

[Message part 1 (text/plain, inline)]

Here we go!

Ludovic Courtès <ludo <at> gnu.org> writes:

> Yes, just one ‘display-hint’ call, using complete sentences and
> paragraphs.  The text should provide an explanation and more importantly
> a hint as to what can done, like “The generated package definition might
> be wrong; please double-check …”.

I made a new version for that.

>
> You can avoid @strong though, because it doesn’t do
> anything useful.

Okay.

>
>> +        (warning
>> +         (G_ "The project name does not appear verbatim in the pypi URI~%"))
>
> I’d make it:
>
>   "project name ~a does not appear verbatim in the PyPI URI~%"

Changed.

>
>>> Also, what about adding a unit test?
>
> [...]
>
>> +(test-equal "If the package goo is released as foo, the importer warns"
>> +  "warning: The project name does not appear verbatim in the pypi URI
>> +hint: The project name is: *goo*
>> +
>> +hint: The pypi URI is: *`https://example.com/foo-1.0.0.tar.gz'*
>> +
>> +hint: The pypi-uri declaration in the generated package might need to be changed.
>> +
>> +"
>> +  (call-with-output-string
>> +    (lambda (port)
>> +      (parameterize ((guix-warning-port port)
>> +                     (current-error-port port))
>> +        ;; Replace network resources with sample data.
>> +        (mock ((guix import utils) url-fetch
>> +               (lambda (url file-name)
>> +                 (match url
>
> These two tests are really integration tests: they exercise the whole
> shebang.  I was thinking about having a unit test of just
> ‘find-project-url’, which is less work (and maintenance :-)) and may be
> good enough.

Let’s do both! If you don’t want the integration tests, just delete them
(now or when maintenance becomes too demanding).

>
> Perhaps you can also change one of the existing python-foo tests to
> exercise this bit, for instance by making it “Foo”.

I added a test function that can be used by all integration tests. It’s
not perfect (there’s still a lot of code copied from test to test, and
I’m not innocent there too) but I don’t want to change the other tests
beyond trivial things.

Vivien

[0001-pypi-importer-Convert-to-_-in-pypi-urls-if-needed.patch (text/x-patch, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #22 received at 53395-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Vivien Kraus <vivien <at> planete-kraus.eu>
Cc: 53395-done <at> debbugs.gnu.org
Subject: Re: bug#53395: Fix pypi import for flake8-array-spacing
Date: Wed, 26 Jan 2022 16:20:52 +0100

Hi,

Vivien Kraus <vivien <at> planete-kraus.eu> skribis:

> From 8792367617d288e584a703db38c82c749a067c26 Mon Sep 17 00:00:00 2001
> From: Vivien Kraus <vivien <at> planete-kraus.eu>
> Date: Thu, 20 Jan 2022 20:11:56 +0100
> Subject: [PATCH] pypi importer: Convert - to _ in pypi urls if needed.
>
> * guix/import/pypi.scm (find-project-url): New function.
> (make-pypi-sexp): Use find-project-url.
> * tests/pypi.scm ("Package Foo has a correct pypi-uri of foo"): New test.
> ("If the package goo is released as foo, the importer warns"): New test.

\o/

I took the liberty to remove the integration tests you had added, I
think it’s preferable (especially since there’s a patch refactoring
these bits that Maxime submitted recently…).  I mentioned the other
changes to the commit log and added a copyright line for you.

Let me know if I got anything wrong.

Thank you!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.