Bug 472229 - (PyQwt) Review Request: PyQwt - Python bindings for Qwt
Review Request: PyQwt - Python bindings for Qwt
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-11-19 09:38 EST by Tadej Janež
Modified: 2015-03-22 02:52 EDT (History)
6 users (show)

See Also:
Fixed In Version: 5.1.0-4.fc9
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-02-01 08:00:06 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Tadej Janež 2008-11-19 09:38:57 EST
Spec URL: http://tadej.hicsalta.si/packages/PyQwt.spec
SRPM URL: http://tadej.hicsalta.si/packages/PyQwt-5.1.0-1.fc9.src.rpm

Description:
PyQwt is a set of Python bindings for the Qwt C++ class library which extends
the Qt framework with widgets for scientific and engineering applications. It
provides a widget to plot 2-dimensional data and various widgets to display and
control bounded or unbounded floating point values.
Comment 1 Tadej Janež 2008-11-19 09:42:54 EST
I forgot to mention that this is my first package and I need a sponsor.
Comment 2 Mamoru TASAKA 2008-12-13 12:21:48 EST
Some notes:

* BuildRequires/Requires 
  - build.log shows
------------------------------------------------------------
    80  Failed to import numarray: PyQwt will be build without support for numarray.
    81  Failed to find Numeric2: PyQwt will be build without support for Numeric.
.....
    86   'disable_numarray': False,
    87   'disable_numeric': False,
    88   'disable_numpy': False,
 ------------------------------------------------------------
    These message seems contradictory.
    Perhaps "BuildRequires: python-numarray python-numeric"
    is needed.

  - Also please check if numpy (or python-numarray, python-numeric)
    are also needed for "Requires" (not BuildRequires). 
    For python module related packages, writing a package in 
    "BuildRequires" does not mean that the package is installed
    at runtime.

  - And check if PyQwt-devel does not require "qwt-devel".
    For example, QwtList.sip contains:
-------------------------------------------------------------
    34  %MappedType QList<QwtPickerMachine::Command>
    35  {
    36  %TypeHeaderCode
    37  #include <qlist.h>
    38  #include <qwt_picker_machine.h>
    39  %End
-------------------------------------------------------------

* Timestamps
  - 'INSTALL="install -p"' argument to "make install" does not
    make sense for this package because this makefile is not
    based on autotools but based on python
    (actually when installing files "cp -f" is used)

* Permission of scripts
-------------------------------------------------------------
# non-executable script
chmod 755 %{buildroot}%{python_sitearch}/PyQt4/Qwt5/grace.py
-------------------------------------------------------------
  - If this script is not meant to be called directly by
    users (but is meant to be called internally from programs
    or so), then this script should not have executable
    permission and the shebang should be removed.
Comment 3 Mamoru TASAKA 2009-01-01 10:34:07 EST
ping?
Comment 4 Tadej Janež 2009-01-09 15:51:39 EST
Firstly, thank you for taking the time to review my package and I apologize for not responding to your review earlier.

(In reply to comment #2)
> Some notes:
> 
> * BuildRequires/Requires 
>   - build.log shows
> ------------------------------------------------------------
>     80  Failed to import numarray: PyQwt will be build without support for
> numarray.
>     81  Failed to find Numeric2: PyQwt will be build without support for
> Numeric.
> .....
>     86   'disable_numarray': False,
>     87   'disable_numeric': False,
>     88   'disable_numpy': False,
>  ------------------------------------------------------------
>     These message seems contradictory.
>     Perhaps "BuildRequires: python-numarray python-numeric"
>     is needed.

I would prefer to avoid all this and just disable support for Numarray and Numeric at configure stage. As far as I know, Numpy is deemed as a replacement for both Numarray and Numeric.

>   - Also please check if numpy (or python-numarray, python-numeric)
>     are also needed for "Requires" (not BuildRequires). 
>     For python module related packages, writing a package in 
>     "BuildRequires" does not mean that the package is installed
>     at runtime.

Yes, it is definitely needed at run time, so I added it to "Requires".

>   - And check if PyQwt-devel does not require "qwt-devel".
>     For example, QwtList.sip contains:
> -------------------------------------------------------------
>     34  %MappedType QList<QwtPickerMachine::Command>
>     35  {
>     36  %TypeHeaderCode
>     37  #include <qlist.h>
>     38  #include <qwt_picker_machine.h>
>     39  %End
> -------------------------------------------------------------

Yes, you are right. I added qwt-devel to PyQwt-devel's "Requires".

> * Timestamps
>   - 'INSTALL="install -p"' argument to "make install" does not
>     make sense for this package because this makefile is not
>     based on autotools but based on python
>     (actually when installing files "cp -f" is used)

I don't completely understand how (not) using autotools affects the use of "install -p" option, but I trust you that it is not necessary.

> 
> * Permission of scripts
> -------------------------------------------------------------
> # non-executable script
> chmod 755 %{buildroot}%{python_sitearch}/PyQt4/Qwt5/grace.py
> -------------------------------------------------------------
>   - If this script is not meant to be called directly by
>     users (but is meant to be called internally from programs
>     or so), then this script should not have executable
>     permission and the shebang should be removed.

grace.py could be executed directly but it requires 'grace' package to work.
If the user doesn't have it installed, it just crashes with an error that
'xmgrace' is not found. However, it seems bloated to me to add a new
'Requires: grace' just because some example script needs it.

Also, upstream is a bit confusing about executable scripts, because a similar
example script qplt.py could also have a shebang at the top and be made
executable but it doesn't have it.
I made grace.py executable because I received warning messages from rpmlint
and I decided that would be the simplest solution.
Maybe it is better to make all example scripts consistent and non-executable?
If so, I should remove the shebang from grace.py.

Updated spec file and source package are at
http://tadej.hicsalta.si/packages/
Comment 5 Mamoru TASAKA 2009-01-11 12:52:46 EST
Well, for 5.1.0-2:

* Header files dependency:
  - Well, for example the installed QwtModule.sip contains:
----------------------------------------------------------------
    63  #include <sipQwtQwtArrayDouble.h>
    64  #include <sipQwtQwtArrayInt.h>
    65  #include <sipQwtQwtArrayQwtDoubleInterval.h>
    66  #include <sipQwtQwtArrayQwtDoublePoint.h>
-----------------------------------------------------------------
    However these header files are not installed. Would
    you check if these are ignored?

* About grace.py
  - Well, I checked this file again and actually this file is 
    executable, so it is more correct that this script should simply
    have 0755 permission (while for qplt.py it seems that
    this script cannot be executed directly).

Then:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------
Comment 6 Mamoru TASAKA 2009-01-18 09:29:13 EST
ping?
Comment 7 Mamoru TASAKA 2009-01-25 02:41:19 EST
ping again?
Comment 8 Tadej Janež 2009-01-25 12:35:22 EST
Thank you for taking time to review my fixed package!
And I apologize again for responding so late.

(In reply to comment #5)
> Well, for 5.1.0-2:
> 
> * Header files dependency:
>   - Well, for example the installed QwtModule.sip contains:
> ----------------------------------------------------------------
>     63  #include <sipQwtQwtArrayDouble.h>
>     64  #include <sipQwtQwtArrayInt.h>
>     65  #include <sipQwtQwtArrayQwtDoubleInterval.h>
>     66  #include <sipQwtQwtArrayQwtDoublePoint.h>
> -----------------------------------------------------------------
>     However these header files are not installed. Would
>     you check if these are ignored?

I traced what is happening to these files.
configure/configure.py in PyQwt-5.1.0.tar.gz contains:
---------------------------------------------------------------------
745  # FIXME: sip-4.7 does not generate those include files anymore
746      for name in [os.path.join(tmp_dir, name) for name in [
747          'sipQwtQwtArrayDouble.h',
748          'sipQwtQwtArrayInt.h',
749          'sipQwtQwtArrayQwtDoubleInterval.h',
750          'sipQwtQwtArrayQwtDoublePoint.h',
751          ]]:
752          if not os.path.exists(name):
753              open(name, 'w')
---------------------------------------------------------------------

So these files get created after running 'python configure.py ...', however,
they remain empty.
Furthermore 'make install' doesn't install them anywhere, which would indicate
that they are obsolete.

I think this is an issue that should be handled upstream.
I submitted a bug in sourceforge.net bug tracker:
https://sourceforge.net/tracker2/?func=detail&aid=2534030&group_id=82987&atid=567933

UPDATE: The author of the package responded that this was a rudimentary hack
to maintain backwards compatibility with older SIP versions and he will remove
it in future versions of the package.

> * About grace.py
>   - Well, I checked this file again and actually this file is 
>     executable, so it is more correct that this script should simply
>     have 0755 permission (while for qplt.py it seems that
>     this script cannot be executed directly).

OK, made grace.py executable again.

> Then:
> -------------------------------------------------------------
> NOTE: Before being sponsored:
> 
> This package will be accepted with another few work. 
> But before I accept this package, someone (I am a candidate) 
> must sponsor you.

I would be very glad if you could sponsor me.
 
> When you have submitted a new review request or have pre-reviewed other 
> person's review request, please write the bug number on this bug report 
> so that I can check your comments or review request.

Ok, I did a pre-review of python-webunit package, which is tracked at bug 481272.
Please check its quality and instruct me on my next steps.

I also uploaded updated spec file and source package at
http://tadej.hicsalta.si/packages/
Comment 9 Mamoru TASAKA 2009-01-27 13:05:19 EST
Okay.
- This package itself is now good
- Your pre-review seems good for initial comments

----------------------------------------------------
   This package (PyQwt) is APPROVED by mtasaka
----------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji)".
Now I am sponsoring you.

If you want to import this package into Fedora 9/10, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Removing NEEDSPONSOR.
Comment 10 Tadej Janež 2009-01-27 18:37:39 EST
OK, thanks for sponsoring me!

New Package CVS Request
=======================
Package Name: PyQwt
Short Description: Python bindings for Qwt
Owners: tadej
Branches: F-9 F-10
InitialCC:
Comment 11 Kevin Fenzi 2009-01-28 00:38:14 EST
cvs done.
Comment 12 Mamoru TASAKA 2009-02-01 08:00:06 EST
Closing.
Comment 13 Fedora Update System 2009-03-06 04:40:09 EST
PyQwt-5.1.0-4.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/PyQwt-5.1.0-4.fc10
Comment 14 Fedora Update System 2009-03-27 10:48:20 EDT
PyQwt-5.1.0-4.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 15 Fedora Update System 2009-04-29 08:08:06 EDT
PyQwt-5.1.0-4.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/PyQwt-5.1.0-4.fc9
Comment 16 Fedora Update System 2009-06-15 22:34:03 EDT
PyQwt-5.1.0-4.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 17 Tadej Janež 2011-01-12 05:33:35 EST
Package Change Request
======================
Package Name: PyQwt
New Branches: el6
Owners: tadej
Comment 18 Jason Tibbitts 2011-01-13 12:21:05 EST
Git done (by process-git-requests).
Comment 19 Jiri Kastner 2014-11-24 10:48:34 EST
Package Change Request
======================
Package Name: PyQwt
New Branches: epel7
Owners: tadej
Comment 20 markusN 2015-03-22 02:52:10 EDT
(In reply to Jiri Kastner from comment #19)
> Package Change Request
> ======================
> Package Name: PyQwt
> New Branches: epel7
> Owners: tadej

I have opened a new request for this at #1204451

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