Re: [PATCH 1/2] gnu: Add numpy

[ludo@gnu.org (Ludovic Courtès)]

Federico Beffa <beffa@ieee.org> skribis:

> On Sun, Oct 26, 2014 at 7:56 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Can these manuals be built from source?
>
> I get error messages that matplotlib is missing.  I started looking at
> matplotlib as well, but I've found that, for the TkAgg back-end, it
> needs TKinter which is part of the standard python libraries and it is
> built if, during the build phase, tk is available. The current python
> package does not build that library (no tk).  In fact, I would like to
> add it to be able to move forward with matplotlib :-)

Heh, OK.  Then can you just summarize that in a comment with TODO, to
give the reader an incentive to look at it?  :-)

> For the other back-ends some packages are missing (python-gtk, Qt, ...)
>
> Another annoying problem is that matplotlib needs numpy, thereby
> creating a dependency loop (although only for the documentation).

If it’s only to build the documentation, we should be able to handle it
easily (by first building a documentation-less variant of numpy, for
instance.)

>> If they can but the process is tedious, then it’s OK to leave it that
>> way with a TODO, and also a comment stating what its license is.
>
> I've not seen any license statement specific for the documentation.
> But, given that the source is distributed with the code, can't we just
> assume it is the same license?

Yes.

>>> +    (inputs
>>> +     `(("python" ,python) ; otherwise ld does not find libpython3.3m
>>
>> This is because Python is not added to ‘LIBRARY_PATH’, right?
>>
>> I think this is fixed by this patch:
>>
>>
>>
>> Can you confirm?
>
> Actually it does not.  The "...-python-numpy-1.9.0-guile-builder"
> still shows that only the python-wrapper is imported. The wrapper does
> not include the libraries.

Ah right.  And what if you again remove Python from ‘inputs’, and add

  #:python ,python

to the arguments?

That means it will use the actual Python 3.x package, not the wrapper,
so everything will be visible.  The downside is that there will be no
‘python’ command, only ‘python3’.

Perhaps the right fix will be to change ‘python-wrapper’ to symlink the
‘lib’ sub-directory of ‘python’.

>> As discussed in the other thread, this should probably be the generic
>> (unoptimized) ATLAS here.
>
> I think that as a temporary situation this is OK.  However, in my
> opinion it does not make much sense to have a separate sub-optimal
> package.  I would like to propose to add specific flags to a single
> atlas package and, once the new "local-build" flag will be available,
> we will remove those flags.

Yeah.

After some more thought, I’ve finally bit the bullet:

  1. Commit 77b0ac9 adds the #:substitutable? flag to gnu-build-system.

  2. Commit f15615b uses it for ATLAS.

It may still make sense to have the non-optimized version for those who
do not need performance and do not want to build locally maybe, WDYT?

>> Use the ‘copy-file’ procedure instead of calling ‘cp’.
>
> The reason that made me use 'cp' instead of 'copy-file' is that the
> documentation states that the return value of the latter is not
> specified and I'm still not familiar with Guile exception handling.
> However, I see many places where 'copy-file' is used without any
> 'catch'.  In principle this should never fail.  If it is OK, I will do
> the same.

‘copy-file’ will throw an exception if something goes wrong; since the
exception is uncaught, the build process will fail.

That’s usually the behavior that we want, so I think it’s best.

>>> +         ;; Tests can only be run after the library has been installed and not
>>> +         ;; within the source directory.
>>> +         (alist-cons-after
>>> +          'install 'check
>>> +          (lambda _
>>> +            (with-directory-excursion "/tmp"
>>> +              (zero? (system* "python" "-c" "import numpy; numpy.test()"))))
>>> +          (alist-delete
>>> +           'check
>>> +           %standard-phases))))))
>>
>> Just (alist-replace 'check ...) ?
>
> Actually the check phase needs to be moved after the install one (see
> comment).  That's the reason for this two step approach.

Ah OK.

Thanks!

Ludo’.