Bug 806037

Summary: Review Request: pygrib - Python module for reading and writing GRIB (editions 1 and 2) files
Product: [Fedora] Fedora Reporter: jdekloe <kloe0040>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: josdekloe, kloedej, notting, package-review, tomspur, volker27
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: 2012-09-08 12:26:10 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 806040    
Bug Blocks:    

Description jdekloe 2012-03-22 13:55:10 EDT
Spec URL: http://jdekloe.nl/Fedora/pygrib.spec
SRPM URL: http://jdekloe.nl/Fedora/pygrib-1.9.2-1.fc17.src.rpm
Description:
GRIB is the the World Meterological Organization (WMO) standard for
distributing gridded data. This module contains python interfaces for reading
and writing GRIB data using the ECMWF GRIB API C library, and the NCEP GRIB2
C library, as well as command-line utilites for listing and re-packing GRIB
files.

This package requires another new package: pyproj which is not yet available for Fedora. I'll create a separate review request for that one.

This is my first Fedora package and I am looking for a sponsor.
Comment 1 Volker Fröhlich 2012-08-02 17:09:00 EDT
Please run rpmlint on your packages:

[makerpm@desktop SPECS]$ rpmlint /var/lib/mock/fedora-16-x86_64/result/*.rpm
pygrib.src: W: summary-ended-with-dot C Python module for reading and writing GRIB (editions 1 and 2) files.
pygrib.src: W: spelling-error %description -l en_US gridded -> griddle, derided, ridged
pygrib.src: W: spelling-error %description -l en_US utilites -> utilities, utilizes, utility
pygrib.src:20: W: mixed-use-of-spaces-and-tabs (spaces: line 20, tab: line 4)
pygrib.src: W: invalid-url Source0: http://pygrib.googlecode.com/files/pygrib-1.9.2.tar.gz HTTP Error 404: Not Found
pygrib.x86_64: W: summary-ended-with-dot C Python module for reading and writing GRIB (editions 1 and 2) files.
pygrib.x86_64: W: spelling-error %description -l en_US gridded -> griddle, derided, ridged
pygrib.x86_64: W: spelling-error %description -l en_US utilites -> utilities, utilizes, utility
pygrib.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/g2clib.so g2clib.so()(64bit)
pygrib.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/redtoreg.so redtoreg.so()(64bit)
pygrib.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/pygrib.so pygrib.so()(64bit)
pygrib.x86_64: E: non-standard-executable-perm /usr/lib64/python2.7/site-packages/g2clib.so 0775L
pygrib.x86_64: E: non-standard-executable-perm /usr/lib64/python2.7/site-packages/redtoreg.so 0775L
pygrib.x86_64: E: non-standard-executable-perm /usr/lib64/python2.7/site-packages/pygrib.so 0775L
pygrib.x86_64: W: no-manual-page-for-binary grib_list
pygrib.x86_64: W: no-manual-page-for-binary cnvgrib1to2
pygrib.x86_64: W: no-manual-page-for-binary cnvgrib2to1
pygrib.x86_64: W: no-manual-page-for-binary grib_repack
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_addlocal.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/cmplxpack.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/simunpack.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/compack.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/gridtemplates.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_unpack1.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/jpcunpack.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/specunpack.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_gribend.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/pdstemplates.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/grib2.h
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_addfield.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/jpcpack.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/gridtemplates.h
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_unpack2.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/pngunpack.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/seekgb.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/rdieee.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_unpack3.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_free.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/mkieee.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/comunpack.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_create.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_getfld.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/drstemplates.h
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/pdstemplates.h
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/drstemplates.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_unpack6.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/pngpack.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_unpack7.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_unpack4.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/int_power.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_info.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_addgrid.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/misspack.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/simpack.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/gbits.c
pygrib-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/pygrib-1.9.2/g2clib_src/g2_unpack5.c
3 packages and 0 specfiles checked; 3 errors, 53 warnings.

This package bundles a copy of g2clib. You must delete it in the prep section and BR it. I assume this also obsoletes all the comments about OPENJPEG and the likes.

You should have a ticket for ExcludeArch, see http://fedoraproject.org/wiki/Packaging:Guidelines#Architecture_Build_Failures.

Drop the BuildArch line.
Comment 2 Jos de Kloe 2012-08-23 11:53:50 EDT
A new spec file and srpm is available here:
Spec URL: http://jdekloe.nl/Fedora/pygrib.spec
SRPM URL: http://jdekloe.nl/Fedora/pygrib-1.9.4-1.fc17.src.rpm

I hope most severe issues have been resolved this time.
g2clib_src has been removed, and the *.c files are now being recreated using cython.
Remaining rpmlint warnings are:

pygrib.x86_64: W: spelling-error %description -l en_US Cython -> Python
pygrib.x86_64: W: spelling-error %description -l en_US grib -> brig, grub, grin
pygrib.x86_64: W: spelling-error %description -l en_US gridded -> griddle, derided, ridged
pygrib.x86_64: W: no-manual-page-for-binary cnvgrib2to1
pygrib.x86_64: W: no-manual-page-for-binary cnvgrib1to2
pygrib.x86_64: W: no-manual-page-for-binary grib_list
pygrib.x86_64: W: no-manual-page-for-binary grib_repack
1 packages and 0 specfiles checked; 0 errors, 7 warnings.

The spelling errors, well I just don't agree with these warnings. I think gridded is a proper english word, and grib and Cython are just names. Is there a way to tell the system that these are no spelling mistakes?

The no manual-page warnings I guess should be communicated to upstream?
Comment 3 Thomas Spura 2012-08-24 08:46:22 EDT
Just a few comments for now:
- What is your mail address registered in FAS?
  You are commenting from another address than you opened this bug and have a third one registered in FAS...
  When you'll be sponsored to the packager group, only with that mail address registered in FAS, you'll be able to do reviews here and have other special rights. So please choose for one everywhere :)

- Don't own %{python_sitearch}/ directly. Just the files below it.
- Please be a bit more specific for %{_bindir}/*
- Building seems to be somehow wrong (Just had a brief look...):
  import pygrib
  ImportError: ./pygrib.so: undefined symbol: grib_index_write

  Did you check, if this program works, when installed?
Comment 4 Volker Fröhlich 2012-08-25 16:00:44 EDT
grib_index_write was only defined in version 1.9 of the grib_api. As far as I can see, we have 1.7 in Fedora.

http://www.ecmwf.int/products/data/software/history/grib_api.html -- Search for "grib_index_write"

Did you not specify 'export OPENJPEG_DIR="%{_usr}/"' for a reason? Also BR openjpeg-devel if you decide to do so.

"%{__python} setup.py build" is enough. The flags are used anyway.

You must require the -static provides for g2clib and grib_api.

"If a library you depend on only provides a static version your package can link against it provided that you BuildRequire the *-static subpackage. Packagers in such a situation should be aware that if a shared library becomes available, that you should adjust your package to use the shared library."

See http://fedoraproject.org/wiki/Packaging:Guidelines#Statically_Linking_Executables

(There's a typo in "pre-coocked".)
Comment 5 Jos de Kloe 2012-08-25 16:28:32 EDT
Thanks for reminding me, I decided to use my gmail account but since I am new here it was not clear to me where I should make changes. First I only changed my email for the devel list. Now I also changed it for the FAS system.

Yes ofcourse I did test the module, using mock to build and test on rawhide. 
The following command sequence works on my F17 system:


mock -r fedora-rawhide-x86_64 --init
mock -r fedora-rawhide-x86_64 --install yum
mock -r fedora-rawhide-x86_64 --install proj proj-devel proj-nad
mock -r fedora-rawhide-x86_64 --no-clean \
     --rebuild pyproj-1.9.2-4.r298.fc16.src.rpm
mock -r fedora-rawhide-x86_64 --shell
cd /builddir/build/RPMS/
rpm -i pyproj-1.9.2-4.r298.fc19.x86_64.rpm
^d
mock -r fedora-rawhide-x86_64 --no-clean \
     --rebuild pygrib-1.9.4-1.fc17.src.rpm
mock -r fedora-rawhide-x86_64 --shell
cd /builddir/build/RPMS/
rpm -i pygrib-1.9.4-1.fc19.x86_64.rpm

cd /builddir/build/BUILD/pygrib-1.9.4
python test.py

# ALL TESTS PASSED!!!!

cd
python
>>> import pygrib
>>> f='/builddir/build/BUILD/pygrib-1.9.4/sampledata/reduced_latlon_surface.grib2'
>>> gribs = pygrib.open(f)
>>> for grib in gribs:
...    print grib
1:Significant height of combined wind waves and swell:m (instant):reduced_ll:meanSea:level 0:fcst time 0 hrs:from 200802061200
>>>^d
Comment 6 Jos de Kloe 2012-08-29 16:44:42 EDT
new versions of the SRPM and SPEC file are here:

Spec URL: http://jdekloe.nl/Fedora/pygrib.spec
SRPM URL: http://jdekloe.nl/Fedora/pygrib-1.9.4-2.fc17.src.rpm

@Thomas:
> Don't own %{python_sitearch}/ directly. Just the files below it.
> Please be a bit more specific for %{_bindir}/*

both have been made more specific.

> Building seems to be somehow wrong (Just had a brief look...):
> import pygrib
> ImportError: ./pygrib.so: undefined symbol: grib_index_write
>
>  Did you check, if this program works, when installed?

As said above the module does work on Rawhide.
On Fedora17 it fails because pyproj cannot be build.
pyproj in turn needs proj v4.8.0 which is not yet available for Fedora17, which only is at proj v4.7.0 at the moment.
As soon as proj gets updated to 4.8.0 I'll try to get pygrib to build on Fedora17 as well.

@Volker:

>grib_index_write was only defined in version 1.9 of the grib_api. As far as I >can see, we have 1.7 in Fedora.

current grib_api in Fedora17 has been updated to v1.9.16 so this is not a problem anymore.

>Did you not specify 'export OPENJPEG_DIR="%{_usr}/"' for a reason? Also BR >openjpeg-devel if you decide to do so.

Yes this has a reason. By applying "trial-and-error" I tried to find a minimal amount of enviroment settings needed to build the package. I found that defining JASPER_DIR and PNG_DIR is sufficient.
I also found that OPENJPEG_DIR should NOT be specified. The build fails if it is added.

> "%{__python} setup.py build" is enough. The flags are used anyway.

thanks. I removed the flags.

> You must require the -static provides for g2clib and grib_api.

this has been added as well.

> (There's a typo in "pre-coocked".)

thanks, fixed it.
Comment 7 Volker Fröhlich 2012-08-29 18:06:20 EDT
Building with openjpeg works, locally on F16 as well as on F17: http://koji.fedoraproject.org/koji/taskinfo?taskID=4436093

I missed the newer API version somehow.

I thought about spelling it "pre-cooked"!

To my knowledge, BRing grib_api-static and g2clib-static should be enough. No need to have grib_api-devel and g2clib-devel too.

That comment doesn't make sense:

"# However, pygrib only requires them (which seems to happen automatically)
# it does not BuildRequires them.
# So no explicit BuildRequires is needed for jasper, openjpeg, png or zlib"

Those libraries are linked with the Python module shared object. rpmbuild figures that out and sets Requires accordingly. Devel packages always require the base package. You therefore mostly need to specify devel packages as BR.
Comment 8 jdekloe 2012-09-08 12:26:10 EDT
moved to my other bugzilla account

*** This bug has been marked as a duplicate of bug 855529 ***