Bug 463217 - Review Request: gnuplot-py - Python interface to Gnuplot
Review Request: gnuplot-py - Python interface to Gnuplot
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Orcan Ogetbil
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-22 12:27 EDT by Sergio Pascual
Modified: 2012-10-31 07:00 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-12-08 08:06:22 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
oget.fedora: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Sergio Pascual 2008-09-22 12:27:01 EDT
Spec URL: http://sergiopr.fedorapeople.org/gnuplot-py.spec
SRPM URL: http://sergiopr.fedorapeople.org/gnuplot-py-1.8-1.fc9.src.rpm
Description: 
Gnuplot.py is a Python package that interfaces to gnuplot, the popular 
open-source plotting program. It allows you to use gnuplot from within 
Python to plot arrays of data from memory, data files, or mathematical 
functions.
Comment 1 James Ralston 2008-09-23 18:02:17 EDT
Your spec file is missing at least:

BuildRequires: numpy

The way you've elected to remove the shebang from various files updates the timestamps on those files.  It is (arguably) better to use a method that retains the original timestamps.  E.g.:

for F in demo.py utils.py __init__.py test.py funcutils.py; do
  %{__sed} -i.orig -e 1d ${F}
  touch -r ${F}.orig ${F}
  rm ${F}.orig
done

You should ask upstream to remove the shebangs from the files in question, and note in the spec file that you have done so.  See:

https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
Comment 2 James Ralston 2008-09-23 18:04:15 EDT
(Note that for consistency with the rest of your spec file, my example in comment 1 should have used "%{__rm}", not "rm".)
Comment 3 Sergio Pascual 2008-09-24 04:31:22 EDT
(In reply to comment #1)

Thanks for your ccomments!

> Your spec file is missing at least:
> 
> BuildRequires: numpy
> 

The package builds ok as it is in mock. I think the dependency I have to add is 
Requires: numpy


> The way you've elected to remove the shebang from various files updates the
> timestamps on those files.  It is (arguably) better to use a method that
> retains the original timestamps.  E.g.:
> 
> for F in demo.py utils.py __init__.py test.py funcutils.py; do
>   %{__sed} -i.orig -e 1d ${F}
>   touch -r ${F}.orig ${F}
>   rm ${F}.orig
> done
> 
> You should ask upstream to remove the shebangs from the files in question, and
> note in the spec file that you have done so.  See:
> 
> https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

I see, I will tell upstream and submit a new specfile
Comment 5 Jason Tibbitts 2008-10-03 13:00:18 EDT
This failed to build in mock for me:

Traceback (most recent call last):
  File "setup.py", line 15, in <module>

from __init__ import __version__
  File "/builddir/build/BUILD/gnuplot-py-1.8/__init__.py", line 165, in <module>

from PlotItems import PlotItem, Func, File, Data, GridData
  File "/builddir/build/BUILD/gnuplot-py-1.8/PlotItems.py", line 24, in <module>

import numpy
ImportError
:
No module named numpy

Are you sure you don't need BuildRequires: numpy?  Can you post a link to a successful scratch build of a package that doesn't have it?
Comment 6 Sergio Pascual 2008-10-03 14:07:01 EDT
Oops! Yes, BuildRequires numpy is needed, sorry

Spec URL: http://sergiopr.fedorapeople.org/gnuplot-py.spec
SRPM URL: http://sergiopr.fedorapeople.org/gnuplot-py-1.8-3.fc9.src.rpm
Comment 7 James Ralston 2008-10-14 12:18:59 EDT
Based on upstream's response, I think removing demo.py and test.py from %{buildroot} is the best thing to do.  E.g.:

%install
%{__rm} -fr %{buildroot}
%{__python} setup.py install --root  %{buildroot}
%{__rm} %{buildroot}%{python_sitelib}/Gnuplot/demo.py*
%{__rm} %{buildroot}%{python_sitelib}/Gnuplot/test.py*
Comment 8 Sergio Pascual 2008-10-18 15:09:02 EDT

(In reply to comment #7)
> Based on upstream's response, I think removing demo.py and test.py from
> %{buildroot} is the best thing to do.  E.g.:
> 

That's what I hav done. Additionally I have included html documentation

Spec URL: http://sergiopr.fedorapeople.org/gnuplot-py.spec
SRPM URL: http://sergiopr.fedorapeople.org/gnuplot-py-1.8-4.fc9.src.rpm
Comment 9 Orcan Ogetbil 2008-10-28 23:05:14 EDT
Package is in good condition. My notes:

* Source0 must be given in full URL.

* I would include the test.py and demo.py inside the %doc files, because they
give a nice demonstration about how gnuplot-py should be used. You don't need
to remove shebangs from them. Just do:
   mv %{buildroot}%{python_sitelib}/Gnuplot/demo.py doc/Gnuplot/
   mv %{buildroot}%{python_sitelib}/Gnuplot/test.py doc/Gnuplot/
in the %install section. Or maybe you may want to install them inside an
"examples" directory under %doc.

Otherwise the package is good to go.
Comment 10 Sergio Pascual 2008-11-02 05:24:47 EST
I have put mode.py and test.py in docs. Source0 contains a full url now

Spec URL: http://sergiopr.fedorapeople.org/gnuplot-py.spec
SRPM URL: http://sergiopr.fedorapeople.org/gnuplot-py-1.8-5.fc9.src.rpm
Comment 11 Orcan Ogetbil 2008-11-28 17:57:47 EST
Sorry, I was never notified that you updated the package. Anyways, I found a few issues:

* There are broken links in the %doc html files. For example, in your browser, go to /usr/share/doc/gnuplot-py-1.8/gp_unix.html . The Gnuplot link on the top left is broken, which is the case for pretty much all other %doc files. Can you fix those?

* I also ask you the include the other relevant doc files (*.txt) in the %doc . These .txt files are referred to within the .html files (see Gnuplot.html) and therefore we need to keep them.

You can put those happydoc generated .html files in a subdirectory inside /usr/share/doc/gnuplot-py-1.8/ if you think that having everything in the main %doc directory makes things confusing.

* This python package does not use setuptools. Therefore in order to create the proper python egg, you need to follow the guideline at:
http://fedoraproject.org/wiki/Packaging/Python/Eggs#Providing_Eggs_for_non-setuptools_packages

* You don't need to package:
   /usr/lib/python2.5/site-packages/Gnuplot/setup.py*
   /usr/share/doc/gnuplot-py-1.8/setup.html
files. There are also mac, win32 etc related .py and .html files. Those aren't very necessary either. I leave these up to you.
Comment 12 Sergio Pascual 2008-12-06 08:24:17 EST
Hi, I think this version fixes all the issues. 

Spec URL: http://sergiopr.fedorapeople.org/gnuplot-py.spec
SRPM URL: http://sergiopr.fedorapeople.org/gnuplot-py-1.8-6.fc10.src.rpm
Comment 13 Orcan Ogetbil 2008-12-06 13:27:24 EST
Thanks. All good
---------------------------------------------
This package (gnuplot-py) is APPROVED by oget
---------------------------------------------
Comment 14 Sergio Pascual 2008-12-07 05:17:38 EST
New Package CVS Request
=======================
Package Name: gnuplot-py
Short Description: Python interface to Gnuplot
Owners: sergiopr
Branches: F-10 F-9
InitialCC:
Comment 15 Kevin Fenzi 2008-12-07 19:26:07 EST
cvs done.
Comment 16 Sergio Pascual 2012-10-30 19:15:47 EDT
Package Change Request
======================
Package Name: gnuplot-py
New Branches: el6
Owners: sergiopr
Comment 17 Gwyn Ciesla 2012-10-31 07:00:07 EDT
Git done (by process-git-requests).

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