[PATCH] IPython: Use 'python2-variant'.

[PATCH] IPython: Use 'python2-variant'.

[Ben Woodcroft]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

Hi,

This hopefully should be a pretty straightforward patch, but I just 
wanted to check that I was doing the correct thing this first time. 
Seems to make the code much cleaner.

Does it make sense to go through all the python packages and convert 
them all to the python2-variant style? As I understand the idea is to do 
this one package per commit, which means traversing python dependency 
graph? Is anyone on this?

Thanks,
ben

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-python-ipython-Use-python2-variant.patch --]
[-- Type: text/x-patch; name="0001-gnu-python-ipython-Use-python2-variant.patch", Size: 2795 bytes --]

From 86d94c052a0ff755e28fbd970b71f87dbba022af Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Sat, 16 Apr 2016 23:10:26 +1000
Subject: [PATCH] gnu: python-ipython: Use 'python2-variant'.

* gnu/packages/python.scm (python-ipython)[properties]: New field.
(python2-ipython): Use 'strip-python2-variant'.
---
 gnu/packages/python.scm | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
index 4238965..e6fa127 100644
--- a/gnu/packages/python.scm
+++ b/gnu/packages/python.scm
@@ -12,7 +12,7 @@
 ;;; Copyright © 2015 Eric Dvorsak <eric@dvorsak.fr>
 ;;; Copyright © 2015, 2016 David Thompson <davet@gnu.org>
 ;;; Copyright © 2015, 2016 Leo Famulari <leo@famulari.name>
-;;; Copyright © 2015 Ben Woodcroft <donttrustben@gmail.com>
+;;; Copyright © 2015, 2016 Ben Woodcroft <donttrustben@gmail.com>
 ;;; Copyright © 2015, 2016 Erik Edrosa <erik.edrosa@gmail.com>
 ;;; Copyright © 2015, 2016 Efraim Flashner <efraim@flashner.co.il>
 ;;; Copyright © 2015 Kyle Meyer <kyle@kyleam.com>
@@ -4357,29 +4357,21 @@ without using the configuration machinery.")
 Powerful interactive shells, a browser-based notebook, support for interactive
 data visualization, embeddable interpreters and tools for parallel
 computing.")
-    (license bsd-3)))
+    (license bsd-3)
+    (properties `((python2-variant . ,(delay python2-ipython))))))
 
 (define-public python2-ipython
-  (let ((ipython (package-with-python2 python-ipython)))
+  (let ((parent (package-with-python2
+                 (strip-python2-variant python-ipython))))
     (package
-      (inherit ipython)
+      (inherit parent)
       ;; FIXME: some tests are failing
-      (arguments
-       `(#:tests? #f ,@(package-arguments ipython)))
-      ;; Make sure we use custom python2-NAME packages.
       ;; FIXME: add pyreadline once available.
-      (propagated-inputs
-       `(("python2-terminado" ,python2-terminado)
-         ,@(alist-delete "python-terminado"
-                         (package-propagated-inputs ipython))))
+      (arguments
+       `(#:tests? #f ,@(package-arguments parent)))
       (inputs
-       `(("python2-jsonschema" ,python2-jsonschema)
-         ("python2-mock" ,python2-mock)
-         ("python2-matplotlib" ,python2-matplotlib)
-         ("python2-numpy" ,python2-numpy)
-         ("python2-requests" ,python2-requests)
-         ,@(fold alist-delete (package-inputs ipython)
-                 '("python-jsonschema" "python-matplotlib" "python-numpy" "python-requests")))))))
+       `(("python2-mock" ,python2-mock)
+         ,@(package-inputs parent))))))
 
 (define-public python-isodate
   (package
-- 
2.6.3

Re: [PATCH] IPython: Use 'python2-variant'.

[Leo Famulari]

On Sun, Apr 17, 2016 at 10:54:03AM +1000, Ben Woodcroft wrote:
> Hi,
> 
> This hopefully should be a pretty straightforward patch, but I just wanted
> to check that I was doing the correct thing this first time. Seems to make
> the code much cleaner.
> 
> Does it make sense to go through all the python packages and convert them
> all to the python2-variant style? As I understand the idea is to do this one
> package per commit, which means traversing python dependency graph? Is
> anyone on this?

What do you mean by "all the python packages"? I think that if a plain
and unadorned package-with-python2 works for a package, then we should
not change the package to use the python2-variant system.

The python2-variant system is really for packages that require different
inputs or build arguments for the python-2 and python-3 versions. Some
of these packages were causing problems throughout the dependency graph
before introducing this system. For example, see how Ludo untangled some
knotty code beginning with 00f2bf34. And again in 519e2f4fd.

My plan was to switch remaining "difficult" packages to python2-variant
when they started causing problems, or when we were going to be changing
a package anyways.

But, if you want to make the changes and check that all the affected
packages still work, then I don't see a reason not to do it :)

By the way, here is the bug report on this topic:
http://bugs.gnu.org/22437

>  (define-public python2-ipython
> -  (let ((ipython (package-with-python2 python-ipython)))
> +  (let ((parent (package-with-python2
> +                 (strip-python2-variant python-ipython))))

I prefer to the upstream name (ipython) rather than "parent". Some
people use "base". I'd rather not introduce a 3rd option ;)

Otherwise it looks okay, assuming you've tested that the referring
packages still work as expected.

Re: [PATCH] IPython: Use 'python2-variant'.

[Ben Woodcroft]

On 19/04/16 05:07, Leo Famulari wrote:
> On Sun, Apr 17, 2016 at 10:54:03AM +1000, Ben Woodcroft wrote:
>> Hi,
>>
>> This hopefully should be a pretty straightforward patch, but I just wanted
>> to check that I was doing the correct thing this first time. Seems to make
>> the code much cleaner.
>>
>> Does it make sense to go through all the python packages and convert them
>> all to the python2-variant style? As I understand the idea is to do this one
>> package per commit, which means traversing python dependency graph? Is
>> anyone on this?
> What do you mean by "all the python packages"? I think that if a plain
> and unadorned package-with-python2 works for a package, then we should
> not change the package to use the python2-variant system.
>
> The python2-variant system is really for packages that require different
> inputs or build arguments for the python-2 and python-3 versions. Some
> of these packages were causing problems throughout the dependency graph
> before introducing this system. For example, see how Ludo untangled some
> knotty code beginning with 00f2bf34. And again in 519e2f4fd.
>
> My plan was to switch remaining "difficult" packages to python2-variant
> when they started causing problems, or when we were going to be changing
> a package anyways.
>
> But, if you want to make the changes and check that all the affected
> packages still work, then I don't see a reason not to do it :)
>
> By the way, here is the bug report on this topic:
> http://bugs.gnu.org/22437

Re-reading that somehow now makes a lot more sense. I'm glad we don't 
have to change everything, apologies for the noise.

>>   (define-public python2-ipython
>> -  (let ((ipython (package-with-python2 python-ipython)))
>> +  (let ((parent (package-with-python2
>> +                 (strip-python2-variant python-ipython))))
> I prefer to the upstream name (ipython) rather than "parent". Some
> people use "base". I'd rather not introduce a 3rd option ;)

Fair point. I'd prefer "base" since then copy and paste works better, 
which is especially useful if the addition of setuptools as a native 
input is the only difference.

> Otherwise it looks okay, assuming you've tested that the referring
> packages still work as expected.

I'll send an updated patch as part of a series I'm preparing.

Thanks for your explanations.
ben

Re: [PATCH] IPython: Use 'python2-variant'.

[Leo Famulari]

On Tue, Apr 19, 2016 at 07:15:25PM +1000, Ben Woodcroft wrote:
> On 19/04/16 05:07, Leo Famulari wrote:
> >>  (define-public python2-ipython
> >>-  (let ((ipython (package-with-python2 python-ipython)))
> >>+  (let ((parent (package-with-python2
> >>+                 (strip-python2-variant python-ipython))))
> >I prefer to the upstream name (ipython) rather than "parent". Some
> >people use "base". I'd rather not introduce a 3rd option ;)
> 
> Fair point. I'd prefer "base" since then copy and paste works better, which
> is especially useful if the addition of setuptools as a native input is the
> only difference.

Good point! I hadn't thought of that benefit of using 'base'.

Re: [PATCH] IPython: Use 'python2-variant'.

[Hartmut Goebel]

[-- Attachment #1.1: Type: text/plain, Size: 864 bytes --]

> On Sun, Apr 17, 2016 at 10:54:03AM +1000, Ben Woodcroft wrote:
>>  (define-public python2-ipython
>> -  (let ((ipython (package-with-python2 python-ipython)))
>> +  (let ((parent (package-with-python2
>> +                 (strip-python2-variant python-ipython))))

I wonder why this "strip-python2-variant" is not integrated into
package-with-python2?

Having it separately IMHO is of no use, it just adds additional code to
be copy&pasted.

-- 
Schönen Gruß
Hartmut Goebel
Dipl.-Informatiker (univ), CISSP, CSSLP, ISO 27001 Lead Implementer
Information Security Management, Security Governance, Secure Software
Development

Goebel Consult, Landshut
http://www.goebel-consult.de

Blog: http://www.goebel-consult.de/blog/feiertagsarbeit-bei-teletrust
Kolumne:
http://www.cissp-gefluester.de/2012-09-steht-ein-manta-fahrer-vor-der-uni


[-- Attachment #1.2: Type: text/html, Size: 2030 bytes --]

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2430 bytes --]

Re: [PATCH] IPython: Use 'python2-variant'.

[Hartmut Goebel]

> On Sun, Apr 17, 2016 at 10:54:03AM +1000, Ben Woodcroft wrote:
>>  (define-public python2-ipython
>> -  (let ((ipython (package-with-python2 python-ipython)))
>> +  (let ((parent (package-with-python2
>> +                 (strip-python2-variant python-ipython))))

I wonder why this "strip-python2-variant" is not integrated into
package-with-python2?

Having it separately IMHO is of no use, it just adds additional code to
be copy&pasted.


-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

‘package-with-python2’ and ‘strip-python2-variant’

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 582 bytes --]

Hartmut Goebel <h.goebel@goebel-consult.de> skribis:

>> On Sun, Apr 17, 2016 at 10:54:03AM +1000, Ben Woodcroft wrote:
>>>  (define-public python2-ipython
>>> -  (let ((ipython (package-with-python2 python-ipython)))
>>> +  (let ((parent (package-with-python2
>>> +                 (strip-python2-variant python-ipython))))
>
> I wonder why this "strip-python2-variant" is not integrated into
> package-with-python2?
>
> Having it separately IMHO is of no use, it just adds additional code to
> be copy&pasted.

You’re right, we can eventually apply this patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2008 bytes --]

diff --git a/guix/build-system/python.scm b/guix/build-system/python.scm
index 326e6fd..3b775d6 100644
--- a/guix/build-system/python.scm
+++ b/guix/build-system/python.scm
@@ -68,7 +68,8 @@ extension, such as '.tar.gz'."
     (module-ref python 'python-2)))
 
 (define* (package-with-explicit-python python old-prefix new-prefix
-                                       #:key variant-property)
+                                       #:key
+                                       variant-property)
   "Return a procedure of one argument, P.  The procedure creates a package with
 the same fields as P, which is assumed to use PYTHON-BUILD-SYSTEM, such that
 it is compiled with PYTHON instead.  The inputs are changed recursively
@@ -130,19 +131,23 @@ pre-defined variants."
 
   transform)
 
+(define (strip-python2-variant p)
+  "Remove the 'python2-variant' property from P."
+  (package
+    (inherit p)
+    (properties (alist-delete 'python2-variant (package-properties p)))))
+
 (define package-with-python2
   ;; Note: delay call to 'default-python2' until after the 'arguments' field
   ;; of packages is accessed to avoid a circular dependency when evaluating
   ;; the top-level of (gnu packages python).
-  (package-with-explicit-python (delay (default-python2))
+  (compose (package-with-explicit-python (delay (default-python2))
                                          "python-" "python2-"
-                                #:variant-property 'python2-variant))
+                                         #:variant-property 'python2-variant)
 
-(define (strip-python2-variant p)
-  "Remove the 'python2-variant' property from P."
-  (package
-    (inherit p)
-    (properties (alist-delete 'python2-variant (package-properties p)))))
+           ;; Since this is the user-facing procedure, we always want to strip
+           ;; the 'python2-variant' property.
+           strip-python2-variant))
 
 (define* (lower name
                 #:key source inputs native-inputs outputs system target

[-- Attachment #3: Type: text/plain, Size: 616 bytes --]


However, before this is possible, we must first change all patterns like:

  (define-public python-netaddr …)

  (define-public python2-netaddr
    (package-with-python2 python-netaddr))

to:

  (define-public python-netaddr
    (package
      ;; …
      (properties `((python2-variant . ,(delay python2-netaddr))))))

  (define-public python2-netaddr
    (package-with-python2 (strip-python2-variant python-netaddr)))

and make sure (with “guix build python2-netaddr -d” and similar) that
the changes have no impact on the resulting derivation.

Volunteers?  :-)

Thanks,
Ludo’.