Bug 234121 - Review Request: PyOpenGL - Python bindings for OpenGL
Review Request: PyOpenGL - Python bindings for OpenGL
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
: 221031 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-27 04:06 EDT by Hans de Goede
Modified: 2007-11-30 17:12 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-04-02 02:55:37 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mtasaka: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Hans de Goede 2007-03-27 04:06:38 EDT
Spec URL: http://people.atrpms.net/~hdegoede/PyOpenGL.spec
SRPM URL: http://people.atrpms.net/~hdegoede/PyOpenGL-3.0.0-0.1.a6.fc7.src.rpm
Description:
Python bindings for OpenGL

---

Notice to reviewers, yes this is an alpha version and the website also has a
shiny 2.x stable version, unfortunately the 2.x version contains parts under the
SGI Free B license, which is not a free license.  3.x is a rewrite without any
license issues (not the reason for the rewrite, but a nice side effect!). Spot
has audited 3.x and its fine license wise.
Comment 1 Mamoru TASAKA 2007-03-27 05:28:48 EDT
Would you discuss with bug 221031 and bug 225118 ?
Comment 2 Hans de Goede 2007-03-27 07:38:32 EDT
(In reply to comment #1)
> Would you discuss with bug 221031 and bug 225118 ?

Woops, done. I'm leaving this open for now. I will close it once its clear with
which package we will move forward, and in which review bug that will get reviewed.
Comment 3 Hans de Goede 2007-03-28 14:12:23 EDT
*** Bug 221031 has been marked as a duplicate of this bug. ***
Comment 4 Hans de Goede 2007-03-28 14:14:21 EDT
The other 2 reviews have both been closed by their respective reporters in favor
of this one. So anyone interested please review this.
Comment 5 Mamoru TASAKA 2007-03-29 04:15:31 EDT
Well, for 3.0.0-0.1.a6:

* Executable permission vs shebang
---------------------------------------------------
chmod +x $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/Tk/__init__.py \
  $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glgetfloat_leak.py \
  $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/tests.py \
  $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glutinit_0args.py \
  $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glut_function_predef.py
---------------------------------------------------
  - Well, please make it sure:
    * If these scripts are expected to be called from other python
      scripts and are not expected to be called "directly" by user
      (well, "directly" used herein may be ambiguous), then
      having shebang is wrong and these files should _not_ have
      executable permission.
    * If opposite, i.e. these scripts are meant to be called directly
      by user, then these scripts should have executable permission.

    Usually the case is former.

* Python related dependency
  - Well, would you explain why the Requires for this package is
    enough by the following?
---------------------------------------------------
Requires:       python-numeric python-setuptools
---------------------------------------------------

    I usually check python related dependency by following. For this
    package, it returns many dependency, such as
    wxPython tkinter python-imaging pygame ... 

---------------------------------------------------
$ grep 'import ' `rpm -ql PyOpenGL | grep py$` | sed -e 's|^.*:||' | sed -e
's|^[ \t][ \t]*||' | sort | uniq
---------------------------------------------------


----------------------------------------------------
  Again, I would appreciate it if you would review
  either of my review request (from bug 233423 to bug 233426), thanks.
Comment 6 Hans de Goede 2007-03-29 06:22:07 EDT
(In reply to comment #5)
> Well, for 3.0.0-0.1.a6:
> 
> * Executable permission vs shebang
> ---------------------------------------------------
> chmod +x $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/Tk/__init__.py \
>   $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glgetfloat_leak.py \
>   $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/tests.py \
>   $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glutinit_0args.py \
>   $RPM_BUILD_ROOT%{python_sitelib}/OpenGL/tests/test_glut_function_predef.py
> ---------------------------------------------------
>   - Well, please make it sure:
>     * If these scripts are expected to be called from other python
>       scripts and are not expected to be called "directly" by user
>       (well, "directly" used herein may be ambiguous), then
>       having shebang is wrong and these files should _not_ have
>       executable permission.
>     * If opposite, i.e. these scripts are meant to be called directly
>       by user, then these scripts should have executable permission.
> 
>     Usually the case is former.
> 

I will look into this.

> * Python related dependency
>   - Well, would you explain why the Requires for this package is
>     enough by the following?
> ---------------------------------------------------
> Requires:       python-numeric python-setuptools
> ---------------------------------------------------
> 
>     I usually check python related dependency by following. For this
>     package, it returns many dependency, such as
>     wxPython tkinter python-imaging pygame ... 
> 
> ---------------------------------------------------
> $ grep 'import ' `rpm -ql PyOpenGL | grep py$` | sed -e 's|^.*:||' | sed -e
> 's|^[ \t][ \t]*||' | sort | uniq
> ---------------------------------------------------
> 

Thanks, good tip, okay, so that gives me the following list of (non standard /
not already required) modules:
Tkinter (+ Dialog)
pygame
wxPython
Image

Correct?

Going through them one by one:
Image -> python-imaging, not needed, I've packaged up PyOpenGL for use with
  glchess, and that works fine without python-imaging, some texture loading 
  utility functions may need this, but I think its best to them make the 
  packages using those functions require python-imaging, to me PyOpenGL is an
  opengl wrapper and as such should require those bits which are absolutely 
  necessary todo that task.

pygame:
[hans@localhost glchess]$ grep -rl pygame /usr/lib/python2.5/site-packages/OpenGL
/usr/lib/python2.5/site-packages/OpenGL/tests/test_glgetfloat_leak.py
/usr/lib/python2.5/site-packages/OpenGL/tests/test_glgetfloat_leak.pyc
/usr/lib/python2.5/site-packages/OpenGL/tests/test_glgetfloat_leak.pyo
/usr/lib/python2.5/site-packages/OpenGL/tests/tests.py
/usr/lib/python2.5/site-packages/OpenGL/tests/testing_context.py
/usr/lib/python2.5/site-packages/OpenGL/tests/testing_context.pyc
/usr/lib/python2.5/site-packages/OpenGL/tests/testing_context.pyo
/usr/lib/python2.5/site-packages/OpenGL/tests/tests.pyc
/usr/lib/python2.5/site-packages/OpenGL/tests/tests.pyo

I've tried nuking the tests dir, but then using PyOpenGL fails with an import
error, so some bits are needed. Since pygame is only needed for tests I don't
want to require it.


tkinter:
[hans@localhost glchess]$ grep -irl Tkinter /usr/lib/python2.5/site-packages/OpenGL
/usr/lib/python2.5/site-packages/OpenGL/Tk/__init__.pyc
/usr/lib/python2.5/site-packages/OpenGL/Tk/__init__.py
/usr/lib/python2.5/site-packages/OpenGL/Tk/__init__.pyo

IOW only needed by applications which actually want to use the TK integration,
those should require tkinter themselves


wxpython:
[hans@localhost glchess]$ grep -irl wxPython /usr/lib/python2.5/site-packages/OpenGL
/usr/lib/python2.5/site-packages/OpenGL/tests/test_nolights.py
/usr/lib/python2.5/site-packages/OpenGL/tests/test_nolights.pyc
/usr/lib/python2.5/site-packages/OpenGL/tests/test_nolights.pyo
/usr/lib/python2.5/site-packages/OpenGL/tests/test_reported_leak.py
/usr/lib/python2.5/site-packages/OpenGL/tests/test_reported_leak.pyc
/usr/lib/python2.5/site-packages/OpenGL/tests/test_reported_leak.pyo

Idem as pygame


---

Before continuing I would like to hear from you what you think about this. 
I've removed all 4 of the above from my system and glchess still works fine, IOW
these really are optional, and thus in my vision should not be required.
Comment 7 Mamoru TASAKA 2007-03-29 10:03:07 EDT
Well, what happens if you completely drop
all files under Tk/ and tests/ temporally?

If no problems ocurr, it does make sense to split such a
usually-unneeded scripts (as python splits tkinter part).

If some problems may arise, would you tell us?
If it can be fixed with easy, then we can think
the case above.
Comment 8 Hans de Goede 2007-03-29 10:58:07 EDT
(In reply to comment #7)
> Well, what happens if you completely drop
> all files under Tk/ and tests/ temporally?
> 

As said I tried removing tests before, but that caused an error, however I tried
again and now it works, so I guess I messed up last time :)

Removing Tk also gives no problems.

Suggested solution:
Remove the tests dir, removing the need for python-imaging, pygame and wxPython

Put the tk support in its own subpackage and make that require tkinter.

Agreed?
Comment 9 Mamoru TASAKA 2007-03-29 11:03:40 EDT
Well, IMO:

* If you think tests scripts are somewhat useful, then
  you can create test subpackage (postgresql seems to
  have test package). 
  If you think that these scripts are not needed, then
  remove them.
  I leave this as how you judge.

* I agree with creating Tk subpackage.
Comment 10 Hans de Goede 2007-03-29 14:22:12 EDT
I've chosen to not package to tests. All mentioned issues are fixed, new version
here:
Spec URL: http://people.atrpms.net/~hdegoede/PyOpenGL.spec
SRPM URL: http://people.atrpms.net/~hdegoede/PyOpenGL-3.0.0-0.2.a6.fc7.src.rpm

I'll review the python bindings for mecab as time allows. I'm afraid I can't
help with the ruby bindings, as I know absolutely nothing about Ruby, it might
be best to post to the list for this, or maybe there is a Ruby SIG?
Comment 11 Mamoru TASAKA 2007-03-29 15:04:33 EDT
(In reply to comment #10)
> I've chosen to not package to tests. All mentioned issues are fixed, new version
> here:
> Spec URL: http://people.atrpms.net/~hdegoede/PyOpenGL.spec
> SRPM URL: http://people.atrpms.net/~hdegoede/PyOpenGL-3.0.0-0.2.a6.fc7.src.rpm
> 

I check this after I once take a rest...

> I'll review the python bindings for mecab as time allows. I'm afraid I can't
> help with the ruby bindings, as I know absolutely nothing about Ruby, it might
> be best to post to the list for this, or maybe there is a Ruby SIG?

"Ruby SIG" means the "guideline for ruby package"? If so,
there is.

http://fedoraproject.org/wiki/Packaging/Ruby
Comment 12 Mamoru TASAKA 2007-03-30 12:39:49 EDT
Well, for -0.2:

* Again python related dependency.
  - Please check the dependency for python-numarray

Other things are okay.
Comment 13 Hans de Goede 2007-03-30 14:07:21 EDT
(In reply to comment #12)
> Well, for -0.2:
> 
> * Again python related dependency.
>   - Please check the dependency for python-numarray
> 
> Other things are okay.

First of all, thanks for all the hard work!

I'm not such a python expert, anything specific I can grab for to see which
parts need it? For the record I don't have it installed:
[hans@shalem ~]$ rpm -q python-numarray
package python-numarray is not installed

And PyOpenGL works fine.
Comment 14 Mamoru TASAKA 2007-03-30 14:24:15 EDT
Well, this script.

/usr/lib/python2.5/site-packages/OpenGL/arrays/numarrays.py
--------------------------------------------
     1  """NumArray implementation of the OpenGL-ctypes array interfaces
     2  """
     3  REGISTRY_NAME = 'numarray'
     4  try:
     5          import numarray
     6  except ImportError, err:
     7          raise ImportError( """No numarray module present: %s"""%(err))
     8  import operator
     9  
---------------------------------------------

However, it seems that this script is not called by
any other scripts in PyOpenGL.

Since I think it is too strict to create subpackages only
for this script and I don't think this script should be
removed (perhaps someone may use this), so I think
it is okay to leave this.

----------------------------------------------
   This package (PyOpenGL) is APPROVED by me.
Comment 15 Hans de Goede 2007-03-30 14:27:26 EDT
New Package CVS Request
=======================
Package Name:      PyOpenGL
Short Description: Python bindings for OpenGL
Owners:            j.w.r.degoede@hhs.nl
Branches:          FC-6 devel
InitialCC:         <empty>

Comment 16 Hans de Goede 2007-04-02 02:55:37 EDT
Many thanks for the review!

Imported and build, closing.

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