Subject: Re: bug#25723: [PATCH] gnu: Add python-cycler.
From: Marius Bakke <[email-masked]>
To: Ricardo Wurmus <[email-masked]>
Cc: [email-masked]
Date: Sun, 19 Feb 2017 22:49:47 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi Ricardo,

Thanks a lot for the feedback! 

Ricardo Wurmus <[email-masked]> writes:

>> +    (propagated-inputs
>> +     `(("python-six" ,python-six)))
>
> Is python-six really required at runtime?  Since you offer a variant for
> Python 2, should python-six only be used with one of these variants?

python-six is imported unconditionally by cycler.py. I believe this is
common practice, and the "six" class offers compatibility "both ways",
not just e.g. 2->3.

>> +    (home-page "http://matplotlib.org/cycler/")
>> +    (synopsis "Composable kwarg iterator")
>
> I assume that “kwarg” is a generally known term in Python, but maybe it
> is not.  Could this be replaced with “keyword argument”?

Yes, that is better, thanks!

>
>> +    (description
>> +     "When using @code{matplotlib} and plotting more than one line, it is
>> +common to want to be able to cycle over one or more artist styles; but the
>> +plotting logic can quickly become involved.
>> +
>> +To address this and enable easy cycling over arbitrary @code{kwargs}, the
>> +@code{Cycler} class was developed.")
>
> I would remove the empty line.
>
>> +    (license license:bsd-3)))
>> +
>> +(define-public python2-cycler
>> +  (package-with-python2 python-cycler))
>> +
>
> See question above about python-six.
> Other than that it looks good to me.  Thank you!

Committed with these changes, thank you!