Bug 554464 (python-pebl) - Review Request: python-pebl - Python Environment for Bayesian Learning
Summary: Review Request: python-pebl - Python Environment for Bayesian Learning
Keywords:
Status: CLOSED ERRATA
Alias: python-pebl
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Thomas Spura
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 744588
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-01-11 19:26 UTC by Tadej Janež
Modified: 2013-04-12 14:13 UTC (History)
6 users (show)

Fixed In Version: python-pebl-1.0.2-7.fc15
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-10-30 00:36:06 UTC
Type: ---
Embargoed:
tomspur: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
Fix two missing BR and provides filtering (921 bytes, patch)
2010-06-12 18:59 UTC, Toshio Ernie Kuratomi
no flags Details | Diff

Description Tadej Janež 2010-01-11 19:26:06 UTC
Spec URL: http://tadej.fedorapeople.org/python-pebl.spec
SRPM URL: http://tadej.fedorapeople.org/python-pebl-1.0.1-1.fc11.src.rpm
Description:

Pebl is a python library and command line application for learning the
structure of a Bayesian network given prior knowledge and observations.

Pebl includes the following features:
- can learn with observational and interventional data
- handles missing values and hidden variables using exact and heuristic methods
- provides several learning algorithms; makes creating new ones simple
- has facilities for transparent parallel execution using several cluster/grid
resources
- calculates edge marginals and consensus networks
- presents results in a variety of formats

I'm in contact with the upstream author about fixing the remaining issues noted in the comments of the .spec file.

Comment 1 Thomas Spura 2010-01-24 03:41:50 UTC
Review:

Good:
- name ok
- license ok
- no static libs
- no *.la




Needswork:
- Requires:	python
  This is added automatically, please delete that.
- There is no python-setuptools-devel any more, please use python-setuptools.
- Please be a bit more explicit in %files to notice, when the egg can't be build:
  e.g.:
  %{python_sitearch}/pebl/
  %{python_sitearch}/pebl-*.egg-info
- %check is missing:
  %{__python} setup.py test

This is currently failing. Maybe you can talk to upstream about this too.


- shipping the testsuite doesn't make sense, please delete that.
  (Maybe %exclude %{python_sitearch}/pebl/test* in %files)

- Missing Requires: numpy, pydot, boto, matplotlib
  What is ipython1.kernel.api? (file src/pebl/taskcontroller/ec2ipy1.py)
  (I hope that's it, please double check.)

- Does it make sense to split the docs into a -docs subpackage?
  The documentation is 6 times bigger than, the main package.
  Maybe a point of view, but I would like that ;) (->SHOULD, no blocker)

Comment 2 Tadej Janež 2010-01-26 22:24:48 UTC
First of all, thank you for reviewing my package!

(In reply to comment #1)
> Review:
> Needswork:
> - Requires: python
>   This is added automatically, please delete that.
> - There is no python-setuptools-devel any more, please use python-setuptools.
> - Please be a bit more explicit in %files to notice, when the egg can't be
> build:
>   e.g.:
>   %{python_sitearch}/pebl/
>   %{python_sitearch}/pebl-*.egg-info

Ok, I fixed those.

> - %check is missing:
>   %{__python} setup.py test
> 
> This is currently failing. Maybe you can talk to upstream about this too.

I asked the upstream author about the test failures and I'm waiting his reply.

> - shipping the testsuite doesn't make sense, please delete that.
>   (Maybe %exclude %{python_sitearch}/pebl/test* in %files)

Why do other Python packages include it? For example: numpy, scipy, python-boto, ...

What should I do with _network.c and _cpd.c, which give me rpmlint errors:
python-pebl.i586: W: devel-file-in-non-devel-package /usr/lib/python2.6/site-packages/pebl/_network.c
python-pebl.i586: W: devel-file-in-non-devel-package /usr/lib/python2.6/site-packages/pebl/_cpd.c

> - Missing Requires: numpy, pydot, boto, matplotlib
>   What is ipython1.kernel.api? (file src/pebl/taskcontroller/ec2ipy1.py)
>   (I hope that's it, please double check.)

Yes, I checked and added the following Requires:
Requires:	numpy
Requires:	pydot
# The Requires below are optional, needed for certain types of functionality
# Requires for creating HTML reports of Pebl results
Requires:	python-matplotlib
Requires:	graphviz
Requires:	python-simplejson
# Requires for the Amazon EC2 Task Controller
Requires:	python-boto

The ipython1.kernel.api is part of IPython1 and is used for the IPython1 Task Controller. IPython1 is the next version of the popular IPython shell that also includes an interactive, clustering solution. Pebl can use IPython1 to execute learners in parallel.

However, IPython1 has ceased since then and the parallel processing has been included in IPython >= 0.9.

The last missing Requires is PyXg, which is the python package for the Apple XGrid Task Controller.

Neither of IPython1 and PyXg are included in Fedora, however, they are not necessary for running the program. Should I make a README.package file describing the situation or should I mention this in the summary of the package. What do you think?

Lastly, rpmlint gives me the following error:
python-pebl.i586: E: explicit-lib-dependency python-matplotlib
What does that mean?

Comment 3 Thomas Spura 2010-02-04 13:57:27 UTC
Cou(In reply to comment #2)
> What should I do with _network.c and _cpd.c, which give me rpmlint errors:
> python-pebl.i586: W: devel-file-in-non-devel-package
> /usr/lib/python2.6/site-packages/pebl/_network.c
> python-pebl.i586: W: devel-file-in-non-devel-package
> /usr/lib/python2.6/site-packages/pebl/_cpd.c

If they are required at runtime, leave them in the main package.
If they don't, I'd propably delete them.

> Neither of IPython1 and PyXg are included in Fedora, however, they are not
> necessary for running the program. Should I make a README.package file
> describing the situation or should I mention this in the summary of the
> package. What do you think?

If the programm does not crash, if they are not installed, I'd mention in the spec file as a comment for you, that this are optional packages.

> Lastly, rpmlint gives me the following error:
> python-pebl.i586: E: explicit-lib-dependency python-matplotlib
> What does that mean?

You can run "rpmlint -I explicit-lib-dependency".
"You must let rpm find the library dependencies by itself. Do not put unneeded
explicit Requires: tags."

So R: python-matplotlib is not needed, but this could be a rpmlint bug, because I don't see it adding the Requires itself:

Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PartialHardlinkSets) <= 4.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
Requires: /usr/bin/python libc.so.6()(64bit) libc.so.6(GLIBC_2.2.5)(64bit) libc.so.6(GLIBC_2.3.4)(64bit) libpthread.so.0()(64bit) libpython2.6.so.1.0()(64bit) python(abi) = 2.6 rtld(GNU_HASH)

Comment 4 Thomas Spura 2010-03-29 21:09:08 UTC
(In reply to comment #3)
> > Lastly, rpmlint gives me the following error:
> > python-pebl.i586: E: explicit-lib-dependency python-matplotlib
> > What does that mean?
> 
> You can run "rpmlint -I explicit-lib-dependency".
> "You must let rpm find the library dependencies by itself. Do not put unneeded
> explicit Requires: tags."
> 
> So R: python-matplotlib is not needed, but this could be a rpmlint bug, because
> I don't see it adding the Requires itself:
> 
> Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <=
> 4.6.0-1 rpmlib(PartialHardlinkSets) <= 4.0.4-1 rpmlib(PayloadFilesHavePrefix)
> <= 4.0-1
> Requires: /usr/bin/python libc.so.6()(64bit) libc.so.6(GLIBC_2.2.5)(64bit)
> libc.so.6(GLIBC_2.3.4)(64bit) libpthread.so.0()(64bit)
> libpython2.6.so.1.0()(64bit) python(abi) = 2.6 rtld(GNU_HASH)    

I saw something similar (don't know where it was):

When there is 'lib' in the name, rpmlint will complain about, but here requiring python-matplotlib is ok.


By the way: Any progress?

-> Ping

Comment 5 Tadej Janež 2010-04-01 13:26:27 UTC
(In reply to comment #3)
> > What should I do with _network.c and _cpd.c, which give me rpmlint errors:
> > python-pebl.i586: W: devel-file-in-non-devel-package
> > /usr/lib/python2.6/site-packages/pebl/_network.c
> > python-pebl.i586: W: devel-file-in-non-devel-package
> > /usr/lib/python2.6/site-packages/pebl/_cpd.c
> 
> If they are required at runtime, leave them in the main package.
> If they don't, I'd propably delete them.

Ok, I deleted them from the package.

> > Neither of IPython1 and PyXg are included in Fedora, however, they are not
> > necessary for running the program. Should I make a README.package file
> > describing the situation or should I mention this in the summary of the
> > package. What do you think?
> 
> If the programm does not crash, if they are not installed, I'd mention in the
> spec file as a comment for you, that this are optional packages.

I created a README.Fedora file describing the situation with PyXg and IPython1.

(In reply to comment #4)
> 
> When there is 'lib' in the name, rpmlint will complain about, but here
> requiring python-matplotlib is ok.

Ok, I'll just ignore that.

> 
> By the way: Any progress?

Yes, sorry for my late reply.
I'm in contact with the upstream author and he will release version 1.0.2 with all the fixes needed for easier packaging of Pebl in Fedora.

Check out the new .spec and README.Fedora files at http://tadej.fedorapeople.org/. There is also the python-pebl-1.0.2-2.fc12.src.rpm, which contains a "preview" of the pebl-1.0.2.tar.gz.

Comment 6 Tadej Janež 2010-06-02 08:59:19 UTC
Ping?

Comment 7 Thomas Spura 2010-06-02 09:32:12 UTC
(In reply to comment #6)
> Ping?    

Pong. Thanks for the ping!!

There are some rpmlint warnings (just posting the relevant ones, which are not ignorable):
python-pebl.src:56: W: macro-in-comment %check
python-pebl.src:57: W: macro-in-comment %{__python}

-> ignorable, because they remind you to add %check again

python-pebl.src:61: W: macro-in-comment %{buildroot}
python-pebl.src:62: W: macro-in-comment %{python_sitelib}

-> Better prefix an %, so this won't show up anymore


python-pebl.x86_64: W: private-shared-object-provides /usr/lib64/python2.6/site-packages/pebl/_network.so _network.so()(64bit)
python-pebl.x86_64: W: private-shared-object-provides /usr/lib64/python2.6/site-packages/pebl/_cpd.so _cpd.so()(64bit)

-> A solution for this is e.g. in bug 537983 comment 27.

python-pebl.x86_64: W: unstripped-binary-or-object /usr/lib64/python2.6/site-packages/pebl/_network.so
python-pebl.x86_64: W: unstripped-binary-or-object /usr/lib64/python2.6/site-packages/pebl/_cpd.so

This is a bit strange... Do you have a debuginfo package? I'm wondering, why there isn't one here...

There is /usr/lib64/python2.6/site-packages/pebl/test/
Is that needed at runtime? If not, it would be nice, if you'd delete that (or ask upstream to do so).

Comment 8 Tadej Janež 2010-06-03 14:52:48 UTC
Thanks for a quick reply!

(In reply to comment #7)
> 
> There are some rpmlint warnings (just posting the relevant ones, which are not
> ignorable):
> python-pebl.src:56: W: macro-in-comment %check
> python-pebl.src:57: W: macro-in-comment %{__python}
> 
> -> ignorable, because they remind you to add %check again

I left the %check disabled and added a comment to the .spec file saying why it is currently disabled.

> python-pebl.src:61: W: macro-in-comment %{buildroot}
> python-pebl.src:62: W: macro-in-comment %{python_sitelib}
> 
> -> Better prefix an %, so this won't show up anymore

Ok, fixed.

> python-pebl.x86_64: W: private-shared-object-provides
> /usr/lib64/python2.6/site-packages/pebl/_network.so _network.so()(64bit)
> python-pebl.x86_64: W: private-shared-object-provides
> /usr/lib64/python2.6/site-packages/pebl/_cpd.so _cpd.so()(64bit)
> 
> -> A solution for this is e.g. in bug 537983 comment 27.

Ok, I tried your solution, however, I don't know if I used it correctly.
The output log of rpmbuild still indicates these unnecessary provides:
Finding  Provides: /usr/lib/rpm/find-provides | grep -v -e '_cpd.so|_network.so'
Finding  Requires: /usr/lib/rpm/find-requires | grep -v -e '_cpd.so|_network.so'
Provides: _cpd.so _network.so
Requires(rpmlib): rpmlib(PartialHardlinkSets) <= 4.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(CompressedFileNames) <= 3.0.4-1
Requires: /usr/bin/python libc.so.6 libc.so.6(GLIBC_2.1.3) libc.so.6(GLIBC_2.3.4) libpthread.so.0 libpython2.6.so.1.0

However, rpmlint doesn't give the above 2 warnings anymore. What is happening here?

> python-pebl.x86_64: W: unstripped-binary-or-object
> /usr/lib64/python2.6/site-packages/pebl/_network.so
> python-pebl.x86_64: W: unstripped-binary-or-object
> /usr/lib64/python2.6/site-packages/pebl/_cpd.so
> 
> This is a bit strange... Do you have a debuginfo package? I'm wondering, why
> there isn't one here...

Yes, I see a separate python-pebl-debuginfo-1.0.2-2.fc13.i686.rpm package built on my system. Also, that is why I don't see the above rpmlint error messages.
 
> There is /usr/lib64/python2.6/site-packages/pebl/test/
> Is that needed at runtime? If not, it would be nice, if you'd delete that (or
> ask upstream to do so).    

Ok, I removed test and test.manual from the final rpm file.

See the new .spec file and .src.rpm at http://tadej.fedorapeople.org/.

Comment 9 Thomas Spura 2010-06-08 17:11:10 UTC
I have a debuginfo too now. Seems to was a wrong setup of the cflags here...


I don't know, why that didn't work with deleting the wrong provides...
Maybe you should ask on the devel list or in #fedora-devel in IRC for help (I can't get online there for the next few days, maybe next week(end).)

Comment 10 Toshio Ernie Kuratomi 2010-06-12 01:13:21 UTC
Missing a BuildRequire: numpy

Maybe other things -- try building in koji to see:

koji build --scratch dist-f13 SRPMFILE

Comment 11 Toshio Ernie Kuratomi 2010-06-12 18:59:30 UTC
Created attachment 423536 [details]
Fix two missing BR and provides filtering

Patch adds BuildRequire on numpy

Patch changes the filter string to filter out the extraneous .so's.  The heart of the issue is that quoting was interacting poorly with the provides script.  This particular formulation works but there could be others that are simpler and work as well.

Comment 12 Tadej Janež 2010-06-15 14:01:27 UTC
Toshio, thank you for the patch!

I put a new version of the .spec file and .src.rpm to http://tadej.fedorapeople.org/.

I also did a scratch build of the package in koji and the results are here: http://koji.fedoraproject.org/koji/taskinfo?taskID=2251550

One thing that still bothers me is how to generate the Sphinx documentation during the %build phase. I get numerous warnings of the form:
/builddir/build/BUILD/pebl-1.0.2/docs/src/config.rst:60: (WARNING/2) autodoc can't import/find class 'pebl.config.StringParameter', it reported error: "No module named pebl.config", please check your spelling and sys.path

Which are all valid, because, for example, pebl.config.StringParameter is not in sys.path.
Any ideas how to overcome this?

Comment 13 Toshio Ernie Kuratomi 2010-06-15 14:46:05 UTC
There's a conf.py file that configures sphinx.  In that file, set sys.path to include the directory that has the pebl module in it.

I think this should::

vim docs/src/conf.py

 import sys, os
+
+ sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..', '..'))

# If your extensions are in another directory, add it here.

Comment 14 Thomas Spura 2011-10-06 21:55:25 UTC
Any news here?
Is doc generation working now?

Comment 15 Tadej Janež 2011-10-08 20:29:49 UTC
(In reply to comment #14)
> Any news here?
> Is doc generation working now?

Sorry about the long delay, this completely fell out of my radar.

I applied the Toshio's patch to fix most of the warnings when building the Sphinx documentation.

I put a new version of the .spec file and .src.rpm to
http://tadej.fedorapeople.org/.

I also did a scratch build of the package in koji and the results are here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3414470

$rpmlint python-pebl-1.0.2-5.fc15.src.rpm
python-pebl.src: W: spelling-error %description -l en_US interventional -> intervention, international, Internationale
python-pebl.src: W: invalid-url Source0: http://pebl-project.googlecode.com/files/pebl-1.0.2.tar.gz HTTP Error 404: Not Found
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

I think the first warning can be ignored. The second, however, puzzles me, because I can open the same link in Firefox and download Pebl?

$ rpmlint python-pebl
python-pebl.x86_64: E: explicit-lib-dependency python-matplotlib
python-pebl.x86_64: W: spelling-error %description -l en_US interventional -> intervention, international, Internationale
python-pebl.x86_64: W: no-manual-page-for-binary pebl
1 packages and 0 specfiles checked; 1 errors, 2 warnings.

The first error should be OK, as discussed in comment #4. The two warnings can be ignored for now.

Regards,
Tadej

Comment 16 Thomas Spura 2011-10-09 10:21:43 UTC
(In reply to comment #15)
> I applied the Toshio's patch to fix most of the warnings when building the
> Sphinx documentation.
OK
> I put a new version of the .spec file and .src.rpm to
> http://tadej.fedorapeople.org/.

Can you please provide SPEC/SRPM links like in comment #0 next time?
Thanks.

> I also did a scratch build of the package in koji and the results are here:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=3414470
OK

I get more rpmlint outputs:
$ rpmlint ~/rpmbuild/SRPMS/python-pebl-1.0.2-5.fc15.src.rpm ~/rpmbuild/RPMS/x86_64/python-pebl-*
python-pebl.src: W: spelling-error %description -l en_US interventional -> intervention, international, Internationale
python-pebl.src: W: invalid-url Source0: http://pebl-project.googlecode.com/files/pebl-1.0.2.tar.gz HTTP Error 404: Not Found
python-pebl.x86_64: E: explicit-lib-dependency python-matplotlib
python-pebl.x86_64: W: spelling-error %description -l en_US interventional -> intervention, international, Internationale
python-pebl.x86_64: W: no-manual-page-for-binary pebl
3 packages and 0 specfiles checked; 1 errors, 4 warnings.

But all ignoreable/false positives.
e.g. spectool works, so the link to the source is good.

- Disabling the internal dependency generator is not needed, when using:
  https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Arch-specific_extensions_to_scripting_languages

or:
  https://fedoraproject.org/wiki/User:Tibbs/AutoProvidesAndRequiresFiltering#RPM_4.9_method_.28Fedora_15_and_newer.29

  (depends, where you want to branch later on)

- For a failing test suite you could append "| :" so at least anything else is checked for now. (Maybe it will completely work, when you also add the dependencies)

- ipython1 should be dead, I hope upstream will adopt the new ipython -.-

_____________________________________________

Will approve, when the dependency generator looks fine, test suite is "should".

Comment 17 Tadej Janež 2011-10-09 15:23:45 UTC
(In reply to comment #16)
> - Disabling the internal dependency generator is not needed, when using:
>  
> https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Arch-specific_extensions_to_scripting_languages

OK, I fixed that (I used the first option).

> - For a failing test suite you could append "| :" so at least anything else is
> checked for now. (Maybe it will completely work, when you also add the
> dependencies)

As it turned out, this is a good thing because it uncovered new failed unit tests.
I noted how many tests are failing and added a link to the upstream bug report.

> - ipython1 should be dead, I hope upstream will adopt the new ipython -.-

Yes, I agree.

Links to the updated files:
Spec URL: http://tadej.fedorapeople.org/python-pebl.spec
SRPM URL: http://tadej.fedorapeople.org/python-pebl-1.0.2-6.fc15.src.rpm

I also did a scratch build of the package in koji and the results are here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3416390

Comment 18 Thomas Spura 2011-10-09 16:14:15 UTC
(In reply to comment #17)
> (In reply to comment #16)
> > - Disabling the internal dependency generator is not needed, when using:
> >  
> > https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Arch-specific_extensions_to_scripting_languages
> 
> OK, I fixed that (I used the first option).
OK
> > - For a failing test suite you could append "| :" so at least anything else is
> > checked for now. (Maybe it will completely work, when you also add the
> > dependencies)
> 
> As it turned out, this is a good thing because it uncovered new failed unit
> tests.
> I noted how many tests are failing and added a link to the upstream bug report.

Thanks.
Some are failing because of a missing "testdata5m.txt", when generating the tar ball upstream seems to not include that here.

Some others seem to cause failures in pydot:
  File "/usr/lib/python2.7/site-packages/pydot.py", line 682, in <lambda>
    self.__setattr__('get_'+attr, lambda a=attr : self.__get_attribute__(a))
  File "/usr/lib/python2.7/site-packages/pydot.py", line 594, in __get_attribute__
    defaults = self.get_parent_graph().get_node( default_node_name )

This also makes it impossible to run the tutorials from:
http://packages.python.org/pebl/tutorial.html

Maybe it would be best to file a bug against pydot...

> Links to the updated files:
> Spec URL: http://tadej.fedorapeople.org/python-pebl.spec
> SRPM URL: http://tadej.fedorapeople.org/python-pebl-1.0.2-6.fc15.src.rpm
> 
> I also did a scratch build of the package in koji and the results are here:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=3416390

Thanks.
______________________________________________________

The package/spec looks fine, but I'd not import this, untill the tutorials are working...

Filed as a bug against pydot with a patch (see depends on bug).

______________________________________________________

APPROVED

Comment 19 Tadej Janež 2011-10-09 16:47:00 UTC
(In reply to comment #18)
> 
> The package/spec looks fine, but I'd not import this, untill the tutorials are
> working...

Yes, I agree.
 
> Filed as a bug against pydot with a patch (see depends on bug).

Thanks for filing the bug report and providing a patch!
 
> APPROVED

And thanks for reviewing the package!

Comment 20 Tadej Janež 2011-10-15 09:42:16 UTC
New Package SCM Request
=======================
Package Name: python-pebl
Short Description: Python Environment for Bayesian Learning
Owners: tadej
Branches: f15 f16
InitialCC:

Comment 21 Gwyn Ciesla 2011-10-16 16:47:08 UTC
Git done (by process-git-requests).

Comment 22 Fedora Update System 2011-10-18 07:24:13 UTC
python-pebl-1.0.2-7.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/python-pebl-1.0.2-7.fc15

Comment 23 Fedora Update System 2011-10-18 22:11:47 UTC
python-pebl-1.0.2-7.fc15 has been pushed to the Fedora 15 testing repository.

Comment 24 Fedora Update System 2011-10-30 00:36:06 UTC
python-pebl-1.0.2-7.fc15 has been pushed to the Fedora 15 stable repository.


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