Bug 485607 - Review Request: scitools - A Python library for scientific computing
Review Request: scitools - A Python library for scientific computing
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Susi Lehtola
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-15 03:48 EST by Joseph Smidt
Modified: 2009-03-17 19:45 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-03-17 19:45:11 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
susi.lehtola: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
rpmlint for scitools (8.20 KB, application/octet-stream)
2009-02-17 22:51 EST, Joseph Smidt
no flags Details

  None (edit)
Description Joseph Smidt 2009-02-15 03:48:07 EST
Spec URL: http://jsmidt.fedorapeople.org/SciTools.spec
SRPM URL: http://jsmidt.fedorapeople.org/SciTools-0.4-0.fc10.src.rpm

Description: 
SciTools is a Python package containing lots of useful tools for scientific
computing in Python. The package is built on top of other widely used packages
such as NumPy, SciPy, ScientificPython, Gnuplot, etc.
Webpage: http://code.google.com/p/scitools/
Comment 1 Susi Lehtola 2009-02-15 05:31:43 EST
- Name should be scitools, not SciTools.
 * You need to change also "setup -q" to "setup -q -n SciTools-%{version}"

- Double Requires: gnuplot, remove the latter one.

- Missing BuildRequires: python-devel

- Missing python_sitelib macro, see
http://fedoraproject.org/wiki/Packaging/Python

- Install section should be
%install
rm -rf $RPM_BUILD_ROOT
%{__python} setup.py install --root $RPM_BUILD_ROOT

- Files should be
%{python_sitelib}/*egg-info
%{python_sitelib}/%{name}
instead of
%{_libdir}/*

as the latter version would own system directories, and doesn't work on 64-bit architectures.
Comment 2 Susi Lehtola 2009-02-15 05:45:18 EST
- Missing Requires: ScientificPython.

- Actually you don't need Requires: gnuplot at all, since gnuplot-py already pulls in gnuplot.

- I'd only Requires: numpy, scipy, ScientificPython and gnuplot-py for the base package, and branch the other suggested requirements into an -extras package.
 At the moment it would pull in python-numeric, python-numarray, vtk-python, python-matplotlib, PyX, veusz (and the rest of the suggested requirements once they get into Fedora).
Comment 3 Susi Lehtola 2009-02-15 05:50:00 EST
I don't think I can accept the package even after the aforementioned modifications, since there are files in %{_bindir} that probably shouldn't be there.

Also most of them have really general names, which may clash with other packages.

/usr/bin/_gnuplot.py
/usr/bin/_gnuplot.pyc
/usr/bin/_gnuplot.pyo
/usr/bin/file2interactive.py
/usr/bin/file2interactive.pyc
/usr/bin/file2interactive.pyo
/usr/bin/floatdiff.py
/usr/bin/floatdiff.pyc
/usr/bin/floatdiff.pyo
/usr/bin/floatdiff.verify
/usr/bin/gnuplot.bat
/usr/bin/hgtools.py
/usr/bin/hgtools.pyc
/usr/bin/hgtools.pyo
/usr/bin/profiler.py
/usr/bin/profiler.pyc
/usr/bin/profiler.pyo
/usr/bin/ps2mpeg.py
/usr/bin/ps2mpeg.pyc
/usr/bin/ps2mpeg.pyo
/usr/bin/pyreport
/usr/bin/regression.py
/usr/bin/regression.pyc
/usr/bin/regression.pyo
/usr/bin/subst.py
/usr/bin/subst.pyc
/usr/bin/subst.pyo
/usr/bin/timer.py
/usr/bin/timer.pyc
/usr/bin/timer.pyo
Comment 4 Joseph Smidt 2009-02-15 14:03:27 EST
Thank you for taking the time to review this package.  I have made the changes you have requested.  The new package is found here:

Spec URL: http://jsmidt.fedorapeople.org/scitools.spec
SRPM URL: http://jsmidt.fedorapeople.org/scitools-0.4-0.1.fc10.src.rpm

By your last comment, are you saying scitools is a lost cause? 
Did you want me to remove all files from %{_bindir} which are not executables? 
Should I move all the files in %{_bindir} to %{_bindir}/scitools?  
Should I try to rename the files?  
Is there anything you would suggest?

Also, there was a directory full of examples I put in /usr/share/scitools.
Comment 5 Susi Lehtola 2009-02-15 16:23:48 EST
(In reply to comment #4)
> By your last comment, are you saying scitools is a lost cause? 
> Did you want me to remove all files from %{_bindir} which are not executables? 
> Should I move all the files in %{_bindir} to %{_bindir}/scitools?  
> Should I try to rename the files?  
> Is there anything you would suggest?
> 
> Also, there was a directory full of examples I put in /usr/share/scitools.

No, I suggest you contact upstream and ask them to correct the problem. Moving and/or renaming files should be done only after consultation, since this breaks compatibility with upstream.

I'm not very familiar with Python, and so I don't know if the files need to be in %{_bindir}. My guess is, though, that they should be in the package's sitelib directory instead.

I'd put the examples in the documentation, since people normally look there for documentation :)
Comment 6 Joseph Smidt 2009-02-15 20:16:56 EST
Okay, I have contacted upstream.  In the meantime I have added the -extras package and moved examples to the doc section.  I took the dependencies for the extras package from your suggestions plus the packages mentioned on the website: http://code.google.com/p/scitools/wiki/Installation

Spec URL: http://jsmidt.fedorapeople.org/scitools.spec
SRPM URL: http://jsmidt.fedorapeople.org/scitools-0.4-0.2.fc10.src.rpm
Comment 7 Susi Lehtola 2009-02-16 01:59:58 EST
Okay, so I didn't get everything on the list :P

You are missing python-pmw (goes together with blt).

Remove %doc from extras.

Requires: grace should be requires: pygrace (which pulls in grace). pygrace does not exist in Fedora yet, but it seemed so simple that I made a package (see review request below).

https://bugzilla.redhat.com/show_bug.cgi?id=485617
Comment 8 Joseph Smidt 2009-02-16 03:56:01 EST
Okay, here are the latest files with the +python-pmw grace->pygrace and -%doc in extras changes:

Spec URL: http://jsmidt.fedorapeople.org/scitools.spec
SRPM URL: http://jsmidt.fedorapeople.org/scitools-0.4-0.3.fc10.src.rpm
Comment 9 Joseph Smidt 2009-02-16 12:51:30 EST
Feedback from the upstream author:


"
I've examined the files in the bin directory and removed some of the
too specialized files and also modified/updated some of them.
Now the directory contains

_gnuplot.py             gnuplot.bat             pyreport
file2interactive.py     profiler.py             regression.py
floatdiff.py            ps2mpeg.py              subst.py

All of these, except pyreport, are scripts developed or used in my
"Python for Computational Science" book (a new README file in the bin
directory briefly explains what the remaining files here are used
for).  The files may well go to another bin directory (/usr/local/bin,
for instance). The only problem I can foresee is that they go to a
directory that users do not have in their PATH.

Another possibility is to say that these files (except pyreport) are
for the mentioned book only, and since this book has a special bin
directory I can easily move them there. This is okay with me if
SciTools users do not think they lose much. What they lose is
basically the following:

gnuplot.bat, _gnuplot.py: ability to run Gnuplot on Windows as it is run on Unix

file2interactive.py: quick creation of interactive sessions (maybe more a utility for a book writer...)

profiler.py: trivial front-end, but makes profiling a one-line command

regression.py, floatdiff.py: original tools supporting regression tests with floating-point numbers (i.e., reference results change with hardware)

subst.py: cross-platform sed-like substitution command

pyreport in this dir allows the original pyreport program to work with
scitools.easyviz.

Tell me what you think. An update to v0.6 was just committed to the
svn repository (incl. the updates mentioned above).
"
Comment 10 Joseph Smidt 2009-02-16 13:06:35 EST
Should the solution be move all the /bin files to the doc directory except pyreport and maybe a couple others and have a README in the doc directory saying "These /bin files are needed to run the examples from the book, if you want to run these example move them to somewhere in your $PATH such as your current directory or add this directory to your $PATH"?
Comment 11 Susi Lehtola 2009-02-16 13:27:34 EST
(In reply to comment #10)
> Should the solution be move all the /bin files to the doc directory except
> pyreport and maybe a couple others and have a README in the doc directory
> saying "These /bin files are needed to run the examples from the book, if you
> want to run these example move them to somewhere in your $PATH such as your
> current directory or add this directory to your $PATH"?

Well, that is one option; another would be to put the files in a package-provided dir that is added in the system $PATH.

Looking at the reply for upstream, I'd put the files under %doc, since the package works without them; but this is your decision to make. Just as long as the files don't interfere with other packages.

pyreport is a probable troublemaker, since (as the author stated) there is a program called pyreport, even though it is not in Fedora at the moment.
Comment 12 José Matos 2009-02-16 13:59:43 EST
I will take of the review of pygrace and so I am interested in the progress of this package. :-)

Joseph is this your first package? Are you sponsored?
Comment 13 Joseph Smidt 2009-02-16 14:19:41 EST
This is my first package.  With any luck I will try to have the whole thing completed today. :)   I don't know about being sponsored, but Jussi above is doing a great job helping me get through the packaging process.
Comment 14 Susi Lehtola 2009-02-16 14:39:12 EST
(In reply to comment #13)
> This is my first package.  With any luck I will try to have the whole thing
> completed today. :)   I don't know about being sponsored, but Jussi above is
> doing a great job helping me get through the packaging process.

Well, in that case I can't formally review the package, you need to get a sponsor to do it; see http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_Sponsored
Comment 15 José Matos 2009-02-16 14:43:07 EST
I am sponsor that was why I was asking. :-)
Comment 16 Susi Lehtola 2009-02-16 14:51:09 EST
(In reply to comment #15)
> I am sponsor that was why I was asking. :-)

Well then, that's nice. I didn't check if Joseph was already in the packager database, but the mistakes in the packaging did raise some suspicion in my mind :P
Comment 17 José Matos 2009-02-16 14:58:42 EST
Jussi, please continue the review. You have been doing a fantastic job. :-)
I coordinate all the issues regarding Joseph sponsorship. :-)

Please give your nod when you think that all issues have been dealt.
Comment 18 Susi Lehtola 2009-02-16 15:17:38 EST
(In reply to comment #17)
> Jussi, please continue the review. You have been doing a fantastic job. :-)
> I coordinate all the issues regarding Joseph sponsorship. :-)
> 
> Please give your nod when you think that all issues have been dealt.

Thanks. Okay, if you say so. Reassigning to myself.
Comment 19 José Matos 2009-02-16 15:57:57 EST
Joseph as part of the sponsorship process to show that you understand the packaging review scheme there some steps to be fulfilled.

I will ask you to review (without approving) pygrace (mentioned by Jussi above) and any other package that you choose that is not yet reviewed.

PS: Talking with someone two timezones away while typing in this thread can result in missing words or worse. ;-)

"I am _a_ sponsor..."
"I _will_ coordinate..."
Comment 20 Joseph Smidt 2009-02-16 16:47:06 EST
Okay, what I decided to do was put bin files in the documentation section with a README.bin also in the %doc to explain what is going on. 

Spec URL: http://jsmidt.fedorapeople.org/scitools.spec
SRPM URL: http://jsmidt.fedorapeople.org/scitools-0.4-0.4.fc10.src.rpm
Patch:    http://jsmidt.fedorapeople.org/scitools_bin_README.patch

I will also start reviewing pygrace.
Comment 21 Susi Lehtola 2009-02-16 17:20:25 EST
- Hmm, so README.bin is a new file? Don't supply it as a patch, then, just make it another source.

Also, as the file is related to the Fedora package, it should be named README.Fedora as a distinction to other files that have been provided by upstream.


- "- Moved _bindir to documentation" is sloppy: the system binary directory has not been moved! The entry should be something like: "- Moved files from %%{_bindir} to documentation to prevent clashes." Note the double %% to prevent rpmbuild from expanding the macro in the changelog.


- Your release numbering is a bit unconventional, normally releases are numbered starting from 1 and with integers, not decimals.


- Also, please run rpmlint on the releases you create (both SRPM/spec file and compiled binary RPMS including the debuginfo packages), and paste the output in the bugzilla entry (here). This helps both you and the reviewer.

I will try to do the review tomorrow.
Comment 22 Joseph Smidt 2009-02-16 18:46:52 EST
Okay, I have ran rpmlint and here are the results.  (I see there are quite a few warnings and errors.  I would fix them, but I am confused how. Could I get a hint?)

Spec URL: http://jsmidt.fedorapeople.org/scitools.spec
SRPM URL: http://jsmidt.fedorapeople.org/scitools-0.4-1.fc10.src.rpm


---------------------------------------------------------------------
rpmlint scitools-0.4-1.fc10.src.rpm 
scitools.src: W: no-%build-section
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

---------------------------------------------------------------------
rpmlint scitools-extras-0.4-1.fc10.i386.rpm: 
scitools-extras.i386: W: no-documentation
scitools-extras.i386: E: explicit-lib-dependency python-matplotlib
scitools-extras.i386: E: devel-dependency dx-devel
1 packages and 0 specfiles checked; 2 errors, 1 warnings.

---------------------------------------------------------------------
rpmlint scitools-0.4-1.fc10.i386.rpm 
"scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/slice_demo1.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/slice_demo2.py
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/Gui.py 0644
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/convergencerate.py 0644
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/isosurface_demo3.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/streamline_demo1.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/alternatives/plotdemo2.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/alternatives/plotdemo4.py
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/StringFunction.py 0644
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/plotdemo0.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/plotdemo4.py
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/odesolve.py 0644
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/contourslice_demo1.py
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/ParameterInterface.py 0644
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/plotdemo3.py
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/PrmDictBase.py 0644
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/BoxField.py 0644
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/redirect_io.py 0644
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/streamtube_demo1.py
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/filetable.py 0644
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/Lumpy.py 0644
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/streamribbon_demo2.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/streamline_demo3.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/mesh_demo.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/alternatives/plotdemo1.py
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/_update.py 0644
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/contourslice_demo2.py
scitools.i386: W: wrong-file-end-of-line-encoding /usr/share/doc/scitools-0.4/bin/gnuplot.bat
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/isosurface_demo2.py
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/easyviz/movie.py 0644
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/DrawFunction.py 0644
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/plotdemo1.py
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/multipleloop.py 0644
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/FunctionSelector.py 0644
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/alternatives/plotdemo3.py
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/Regression.py 0644
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/BoxGrid.py 0644
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/NumPyDB.py 0644
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/streamline_demo2.py
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/ppimport.py 0644
scitools.i386: E: non-executable-script /usr/lib/python2.5/site-packages/scitools/CanvasCoords.py 0644
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/streamtube_demo2.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/streamribbon_demo1.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/plottest.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/plotdemo2.py
scitools.i386: W: spurious-executable-perm /usr/share/doc/scitools-0.4/examples/isosurface_demo1.py
scitools.i386: E: no-binary
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/contourslice_demo1.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/contourslice_demo2.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/isosurface_demo1.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/isosurface_demo2.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/isosurface_demo3.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/mesh_demo.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/slice_demo1.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/slice_demo2.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/streamline_demo1.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/streamline_demo2.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/streamline_demo3.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/streamribbon_demo1.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/streamribbon_demo2.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/streamtube_demo1.py /usr/bin/env
scitools.i386: W: doc-file-dependency /usr/share/doc/scitools-0.4/examples/streamtube_demo2.py /usr/bin/env
1 packages and 0 specfiles checked; 21 errors, 41 warnings."
Comment 23 Susi Lehtola 2009-02-17 11:36:41 EST
- You can correct the warning about the spec file by adding a %build section.

- extras: matplotlib is ok, but dx-devel should probably be dx.

- You must add BuildArch: noarch to the spec file to correct the no binary error. (There's nothing architecture specific in the package, so it's enough to build it once, you don't need separate i386, x86_64 etc packages).

José: please correct me if I'm wrong, but AFAIK the non-executable script errors and spurious file perms warnings in files under /usr/lib/python2.5/site-packages/ don't cause any action.

The doc file dependency warnings show, that maybe it's wiser after all to put the examples in /usr/share/%{name}-%{version}/examples. Remember to add a note about the examples to the package readme.
Comment 24 Joseph Smidt 2009-02-17 13:48:08 EST
I have made the above changes so far except moving the examples since I wanted to ask: Is this really necessary.  I know rpmlint is complaining, but end users will want to look to the documentation to see examples.  I know whenever I want examples I look to the documentation.

But if it is needed, I will change it.
Comment 25 Susi Lehtola 2009-02-17 15:54:43 EST
(In reply to comment #24)
> I have made the above changes so far except moving the examples since I wanted
> to ask: Is this really necessary.  I know rpmlint is complaining, but end users
> will want to look to the documentation to see examples.  I know whenever I want
> examples I look to the documentation.
> 
> But if it is needed, I will change it.

For me it's okay if the examples stay in %doc.
Comment 26 Joseph Smidt 2009-02-17 22:51:29 EST
Created attachment 332336 [details]
rpmlint for scitools

Here are the new files with the updates we discussed:

Spec URL: http://jsmidt.fedorapeople.org/scitools.spec
SRPM URL: http://jsmidt.fedorapeople.org/scitools-0.4-2.fc10.src.rpm
Comment 27 José Matos 2009-02-18 07:26:10 EST
OK, removing FE-NEEDSPONSOR (I am sponsoring).
Comment 28 José Matos 2009-02-18 08:37:09 EST
I have adjusted the Summary for the new name.

One small suggestion, replace
Source0:    http://scitools.googlecode.com/files/SciTools-0.4.tar.gz
with
Source0:    http://scitools.googlecode.com/files/SciTools-%{version}.tar.gz

That avoid to edit that line every time there is a new release. This is a personal preference so you are free to ignore it.

Some small nitpicks:

* The spec file uses both %{buildroot} and $RPM_BUILD_ROOT, the guidelines warn against using both. Since there is only one instance of $RPM_BUILD_ROOT I suggest to move that to %{buildroot}.

* In the website and in %description the capitalization of gnuplot is wrong. Clearly this is really (really) a minor point.

I am not sure about the need of an extras sub-package. I would expect that most of the users of this package will have the full set installed. In any case I suggest to move all the dependencies to the main package and only do something about it if there are complains later that this is a problem.
Comment 29 José Matos 2009-02-18 08:41:56 EST
I agree that the examples should stay in %doc (I asked about the same thing in pygrace so... ;-) ).
Comment 30 Susi Lehtola 2009-02-18 09:58:07 EST
(In reply to comment #28)
> I am not sure about the need of an extras sub-package. I would expect that most
> of the users of this package will have the full set installed. In any case I
> suggest to move all the dependencies to the main package and only do something
> about it if there are complains later that this is a problem.

I don't agree; the package should not be bloated. You don't need all of the packages to use SciTools. Also one must remember that there are still people using dialup connections, for whom downloading lots and lots of extra stuff is a problem.

However, you might want to add a note about the extras metapackage to the %description, e.g. "For full functionality you may want to install the scitools-extras package."
Comment 31 Susi Lehtola 2009-02-18 10:03:21 EST
A few notes more:

When you copy the package README, use 'cp -a' instead of 'cp' to preserve the time stamp.

(Also, you might want to add a comment about removing the bindir after installation.)
Comment 32 Susi Lehtola 2009-02-18 10:08:51 EST
The package adheres to the Fedora Packaging and Review Guidelines, and is thus

ACCEPTED.


(You can do the tidying bits mentioned above once you import to CVS.)
Comment 33 José Matos 2009-02-18 11:31:24 EST
(In reply to comment #30)
> 
> I don't agree; the package should not be bloated. You don't need all of the
> packages to use SciTools. Also one must remember that there are still people
> using dialup connections, for whom downloading lots and lots of extra stuff is
> a problem.

It is not so much the fact that I may agree or not with that stance but instead the question to know if the use of a sub-package is the right technical solution to this problem.

> However, you might want to add a note about the extras metapackage to the
> %description, e.g. "For full functionality you may want to install the
> scitools-extras package."

I agree.
Comment 34 Joseph Smidt 2009-02-18 14:57:57 EST
Okay, here is the final version with all the changes:

Spec URL: http://jsmidt.fedorapeople.org/scitools.spec
SRPM URL: http://jsmidt.fedorapeople.org/scitools-0.4-3.fc10.src.rpm

Will request CVS.
Comment 35 Joseph Smidt 2009-02-18 15:00:30 EST
New Package CVS Request
=======================
Package Name: scitools
Short Description: A Python library for scientific computing
Owners: jsmidt
Branches: F-9 F-10 EL-4 EL-5
InitialCC:
Comment 36 Kevin Fenzi 2009-02-18 15:09:56 EST
Joseph: You must create a bugzilla account with your email address that is in the Fedora account system and use that. You cannot use a @fedoraproject.org address as that is only a forward.
Comment 37 Susi Lehtola 2009-02-18 15:32:43 EST
Put the comment about the extras (also) in the main %description, so that users find it.

Also: grace (at the moment) does not exist in EL-4, which means that pygrace is not available in EL-4, which in turn means that scitools can't be installed on EL-4.

You can circumvent this by checking the EPEL repositories to see which of the packages -extras depends on aren't available, and omit those requirements. Example:

%if 0%{?rhel} != 4
Requires: pygrace
%endif
Comment 38 Joseph Smidt 2009-02-18 18:34:06 EST
Dear CVS admins:  This is the account I signed up with as josephsmidt@gmail.com, my Fedora Account Email. (You will find that josephsmidt@gmail.com is taken since it is this account) I have changed the changelog  in the .spec to reflect the gmail address.  I verified with this account that jsmidt@fedoraproject.org also works, and now that is the only email it shows.  I have emailed the bugzilla people about it.

Here are the new sources:

Spec URL: http://jsmidt.fedorapeople.org/scitools.spec
SRPM URL: http://jsmidt.fedorapeople.org/scitools-0.4-4.fc10.src.rpm
Comment 39 Joseph Smidt 2009-02-18 21:43:05 EST
Okay, email is fixed now.
Comment 40 Joseph Smidt 2009-02-19 03:15:13 EST
New Package CVS Request
=======================
Package Name: scitools
Short Description: A Python library for scientific computing
Owners: jsmidt
Branches: F-9 F-10 EL-4 EL-5
InitialCC:
Comment 41 Kevin Fenzi 2009-02-19 12:47:07 EST
Thanks!

cvs done.
Comment 42 Susi Lehtola 2009-03-17 19:45:11 EDT
Joseph: remember to close the Review Request once your package hits the distro. (You can make the update manager do this, when you submit the package.)

Closing.

Note You need to log in before you can comment on or make changes to this bug.