Bug 218195

Summary: Review Request: scipy - array processing for numbers, strings, records, and objects.
Product: [Fedora] Fedora Reporter: Jef Spaleta <jspaleta>
Component: Package ReviewAssignee: José Matos <jamatos>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: rdieter
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-12-07 02:20:16 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779    

Description Jef Spaleta 2006-12-03 02:28:35 UTC
Spec URL: 
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/scipy/scipy.spec

SRPM URL: http://jspaleta.thecodergeek.com/Fedora%20SRPMS/scipy/scipy-0.5.1-2.src.rpm

Description: 
Scipy is a general-purpose array-processing package designed to
efficiently manipulate large multi-dimensional arrays of arbitrary
records without sacrificing too much speed for small multi-dimensional
arrays.  Scipy is built on the Numeric code base and adds features
introduced by numarray as well as an extended C-API and the ability to
create arrays of arbitrary type.

There are also basic facilities for discrete fourier transform,
basic linear algebra and random number generation.

Comment 1 José Matos 2006-12-03 18:22:39 UTC
As stated in the mailing list I will take the review.

Some small questions, why do you do BuildRequires libstdc++? This should work 
without it, no?

I think also the Requires fftw2 is redundant, since rpm will peek the right 
dependency. The same should apply to blas and lapack. Am I missing something 
here?

BTW, %install must have 
rm -rf $RPM_BUILD_ROOT

at its beginning.

Otherwise the spec looks really clean.

Comment 2 José Matos 2006-12-03 20:33:49 UTC
Review for release 2:
* RPM name is OK
* Source scipy-0.5.1.tar.gz is the same as upstream
* Builds fine in mock
* File list looks OK
* License is correct and acceptable (BSD)
* Spec file is written in American English

Needs work:
* The BuildRoot must be cleaned at the beginning of %install
* rpmlint:
W: scipy summary-ended-with-dot Scipy: array processing for numbers, strings, 
records, and objects.

  Please remove the final dot from Summary

There are lots of devel-file-in-non-devel-package warnings, why not to put 
this into a subpackage -devel? OTHO since numpy does not have this separation 
this is not a blocker, I think that this issue should be tackled in both 
packages at the same time.


One other question: Why do you use fftw2 and not fftw version 3? Just curious.

Summary, if you fix the cleaning of %install this package is approved. No need 
to submit a new version I trust you to fix this before importing.

APPROVED (with due conditions met, this not is here just for an easier 
reference)

Comment 3 Jef Spaleta 2006-12-03 22:13:25 UTC
The rpmlint warnings concerning the .h files down in the python site-packages
tree are a matter of interpretation I think. Since these files are not in
/usr/include/python2.4/ I'm not sure there is an expectation that they are to be
built against.  There are many python packages which continue to place .h files
down in the site-packages tree without splitting into a -devel.   

The only python module packages that I am aware of that uses a devel subpackage
install their .h files into the /usr/include/python2.4/ tree.  python-ogg-devel
 for example.  But even that isn't consistently done, python-numarry doesn't
make the effort.  I don't have a problem splitting this stuff off, I just don't
want to set a new packaging policy precedent in the process.

If you could point me to a python module package (ie not a graphical end-user
application) which has .h files down in site-packages and splits out a -devel
subpackage I'd like to look over its spec as a reference.

I fixed the %install section issue and the silly dot at the end of the summar.
Can you tell I'm a couple of months out of practise.  I'm going to play with the
requires and buildrequires a little to see if I can answer the questions.  For
example, I don't know if this will work with fftw version 3....yet.

-jef

Comment 4 José Matos 2006-12-03 23:09:26 UTC
(In reply to comment #3)
> The rpmlint warnings concerning the .h files down in the python 
site-packages
> tree are a matter of interpretation I think. Since these files are not in
> /usr/include/python2.4/ I'm not sure there is an expectation that they are 
to be
> built against.  There are many python packages which continue to place .h 
files
> down in the site-packages tree without splitting into a -devel.   
> 
> The only python module packages that I am aware of that uses a devel 
subpackage
> install their .h files into the /usr/include/python2.4/ tree.  
python-ogg-devel
>  for example.  But even that isn't consistently done, python-numarry doesn't
> make the effort.  I don't have a problem splitting this stuff off, I just 
don't
> want to set a new packaging policy precedent in the process.
> 
> If you could point me to a python module package (ie not a graphical 
end-user
> application) which has .h files down in site-packages and splits out 
a -devel
> subpackage I'd like to look over its spec as a reference.

  You are right. This is not blocking, more like a wish. :-)
 
> I fixed the %install section issue and the silly dot at the end of the 
summar.

  Good, I changed the status of this review to approved.

> Can you tell I'm a couple of months out of practise.  I'm going to play with 
the
> requires and buildrequires a little to see if I can answer the questions.  
For
> example, I don't know if this will work with fftw version 3....yet.

  From the build log:
fft_opt_info:
fftw3_info:
  libraries fftw3 not found in /usr/local/lib
  libraries fftw3 not found in /usr/lib
  fftw3 not found
  NOT AVAILABLE

fftw2_info:
  FOUND:
    libraries = ['rfftw', 'fftw']
    library_dirs = ['/usr/lib64']
    define_macros = [('SCIPY_FFTW_H', None)]
    include_dirs = ['/usr/include']

This was why I asked. :-)

> -jef



Comment 5 Jef Spaleta 2006-12-04 00:22:52 UTC
I take it you did your mock builds on 64bit hardware and had no weird problems
associated /usr/lib/ versus /usr/lib64 ?


-jef


Comment 6 Jef Spaleta 2006-12-04 00:58:54 UTC
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/scipy/scipy-0.5.1-4.fc6.i386.rpm
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/scipy/scipy-0.5.1-4.fc6.src.rpm
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/scipy/scipy.spec

These should address all the comments concerning the unnecessary explicit
requires, and builds against fftw instead of fftw2.

looking more closely at the rpmlint output, I'm getting errors mixed in with the
warnings of the form:
E: scipy non-executable-script
/usr/lib/python2.4/site-packages/scipy/signal/setup.py 0644

There are a number of these and its all due to the existance of
#!/usr/bin/env python

at the top of each file. 

Should I take the time to patch this out to be correct for fedora and to adjust
permissions on these files accordingly.  The real problem is, is that many times
included python files are never meant to be run as executables even though they
have the /usr/bin/whatever header string.

-jef





Comment 7 José Matos 2006-12-04 01:07:23 UTC
(In reply to comment #5)
> I take it you did your mock builds on 64bit hardware and had no weird 
problems
> associated /usr/lib/ versus /usr/lib64 ?

  I always use mock on 64 bits for that reason, and the fact that sitelib != 
sitearch (for python packages) as well.

> -jef

(In reply to comment #6)
> 
> These should address all the comments concerning the unnecessary explicit
> requires, and builds against fftw instead of fftw2.
> 
> looking more closely at the rpmlint output, I'm getting errors mixed in with 
the
> warnings of the form:
> E: scipy non-executable-script
> /usr/lib/python2.4/site-packages/scipy/signal/setup.py 0644
> 
> There are a number of these and its all due to the existance of
> #!/usr/bin/env python
> 
> at the top of each file. 
> 
> Should I take the time to patch this out to be correct for fedora and to 
adjust
> permissions on these files accordingly.  The real problem is, is that many 
times
> included python files are never meant to be run as executables even though 
they
> have the /usr/bin/whatever header string.

  To me those warnings are bogus since I agree with your analysis. I noticed 
them before, while doing the review, but I have ignored them on purpose.

> -jef



Comment 8 Jef Spaleta 2006-12-07 02:20:16 UTC
Okay its now in extras-development. I'll test it from there and then request a
fc6 branching.

I'll be working on enabling some of the sandbox modules as a sandbox subpackage
in the devel tree next.

-jef

Comment 9 José Matos 2006-12-07 02:30:46 UTC
Although I have not said that above I have been testing this module in FC6 
(and so does my wife in FC5) and it works as it is supposed.