Bug 976714

Hi Sergio, 

Check the Requires that repeat in the BuildRequires.
rm -rf %{buildroot} - Remove!

Hi @Sergio, 
- Why you naming the package as python-sklearn?
according to pypy the package is named scikit-learn
https://pypi.python.org/pypi/scikit-learn/
Please rename the package according to the upstream name

About BuildRequires:
python-devel is wrong, In Fedora is correct python2-devel or python3-devel depending of the interpreter that is used
See: http://fedoraproject.org/wiki/Packaging:Python#BuildRequires

- You are filtering the privates so* however you are using the old method for filter

I quote => http://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering
"As of rpm-4.9 (Fedora 15), rpm has a standard method to enable filtering. This page documents that. EPEL5 and 6 do not have a recent enough version of rpm to follow these guidelines. See EPEL:Packaging_Autoprovides_and_Requires_Filtering if your package is to be built there as well." 

rm -rf %{buildroot} is not needed for latests versions of Fedora is only necessary for el5

- explicit-lib-dependency python-matplotlib, rpm find the library dependencies by itself. Do not put unneeded 
please see http://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

the spec file that contains the SRPM differ of the spec publicated here

Diff spec file in url and in SRPM
---------------------------------
--- /home/makerpm/976714-python-sklearn/srpm/python-sklearn.spec        2013-06-22 03:06:43.872642541 -0430
+++ /home/makerpm/976714-python-sklearn/srpm-unpacked/python-sklearn.spec       2013-06-22 03:06:44.486655554 -0430
@@ -47,9 +47,10 @@
 
 %check
-%{__python} setup.py build_ext --inplace
+python setup.py build_ext --inplace
 nosetests sklearn/
  
 %files
 %doc examples/ AUTHORS.rst README.rst
+#%doc examples/ AUTHORS.rst README-py3k.rst README.rst
 %{python_sitearch}/sklearn
 %{python_sitearch}/scikit_learn-*.egg-info

- Please remove the bundle egg in %prep as well
# Remove bundled egg-info
rm -rf foo-info

- the package is arched, so you should do honors compilers flags

please see
➜  ~  rpm -E %{optflags}
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic

Best Regards

New spec and SRPM that addresses the comments 

Spec URL: http://guaix.fis.ucm.es/~spr/python-scikit-learn.spec
SRPM URL: http://guaix.fis.ucm.es/~spr/python-scikit-learn-0.13.1-2.fc19.src.rpm

*** Bug 883489 has been marked as a duplicate of this bug. ***

Hi,

Thanks for packaging it because another module needs it.

IMO I think there is no need to ship README-py3k.rst as this module doesn't support py3 fully.

(In reply to Christopher Meng from comment #5)
> Hi,
> 
> Thanks for packaging it because another module needs it.

You are welcome

> 
> IMO I think there is no need to ship README-py3k.rst as this module doesn't
> support py3 fully.

Yes, that's what I thought when I read README-py3k.rst

I do not need a sponsor. I'm removing the blocking

Please contact with the reviewer and ask if he is willing to review it for now.

Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=5592879

$ rpmlint -i -v *
python-scikit-learn.src: I: checking
python-scikit-learn.src: W: spelling-error %description -l en_US matplotlib -> diplomatic
The value of this tag appears to be misspelled. Please double-check.

python-scikit-learn.src: I: checking-url http://scikit-learn.org/ (timeout 10 seconds)
python-scikit-learn.src: I: checking-url https://pypi.python.org/packages/source/s/scikit-learn/scikit-learn-0.13.1.tar.gz (timeout 10 seconds)
python-scikit-learn.i686: I: checking
python-scikit-learn.i686: W: spelling-error %description -l en_US matplotlib -> diplomatic
The value of this tag appears to be misspelled. Please double-check.

python-scikit-learn.i686: I: checking-url http://scikit-learn.org/ (timeout 10 seconds)
python-scikit-learn.x86_64: I: checking
python-scikit-learn.x86_64: W: spelling-error %description -l en_US matplotlib -> diplomatic
The value of this tag appears to be misspelled. Please double-check.

python-scikit-learn.x86_64: I: checking-url http://scikit-learn.org/ (timeout 10 seconds)
python-scikit-learn-debuginfo.i686: I: checking
python-scikit-learn-debuginfo.i686: I: checking-url http://scikit-learn.org/ (timeout 10 seconds)
python-scikit-learn-debuginfo.x86_64: I: checking
python-scikit-learn-debuginfo.x86_64: I: checking-url http://scikit-learn.org/ (timeout 10 seconds)
python-scikit-learn.spec: I: checking-url https://pypi.python.org/packages/source/s/scikit-learn/scikit-learn-0.13.1.tar.gz (timeout 10 seconds)
python-scikit-learn.spec: W: invalid-url Source0: https://pypi.python.org/packages/source/s/scikit-learn/scikit-learn-0.13.1.tar.gz The read operation timed out
The value should be a valid, public HTTP, HTTPS, or FTP URL.

5 packages and 1 specfiles checked; 0 errors, 4 warnings.


Besides some ignorable spelling errors, nothing of interest. The warning about an invalid source location is false positive, it is downloadable using wget:

$ wget https://pypi.python.org/packages/source/s/scikit-learn/scikit-learn-0.13.1.tar.gz
--2013-07-10 23:09:41--  https://pypi.python.org/packages/source/s/scikit-learn/scikit-learn-0.13.1.tar.gz
Auflösen des Hostnamen »pypi.python.org (pypi.python.org)«... 199.27.76.184, 199.27.76.185
Verbindungsaufbau zu pypi.python.org (pypi.python.org)|199.27.76.184|:443... verbunden.
HTTP-Anforderung gesendet, warte auf Antwort... 200 OK
Länge: 3477412 (3,3M) [application/x-gzip]
In »»scikit-learn-0.13.1.tar.gz«« speichern.

100%[======================================>] 3.477.412   1,42MB/s   in 2,3s   

2013-07-10 23:09:44 (1,42 MB/s) - »»scikit-learn-0.13.1.tar.gz«« gespeichert [3477412/3477412]

Sergio,

  I'm reviewing your package, please check some issues:

- Large documentation must go in a -doc subpackage.
  Note: Documentation size is 2222080 bytes in 280 files.
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation

- Setting Variables with a only one request for line , example:
  BuildRequires: python2-devel 
  BuildRequires: numpy 
  See: https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#BuildRequires

- Review your licence, i believe mix licences in this package. I don't see a BSD license at all.

   "License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "BSD (3 clause)", "BSD (2 clause)", "Unknown or generated". 492 files
     have unknown license. Detailed output of licensecheck in
     /home/marcelo.barbosa/rpmbuild/SOURCES/reviews/976714-python-scikit-
     learn/licensecheck.txt"

  See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
  See: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios 

- Package consistently uses macro is (instead of hard-coded directory names).
  See: https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Macros

- Please see devel files unversioned:
Unversioned so-files
--------------------
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/__check_build/_check_build.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/_hmmc.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/cluster/_hierarchical.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/cluster/_k_means.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/datasets/_svmlight_format.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/ensemble/_gradient_boosting.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/feature_extraction/_hashing.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/linear_model/cd_fast.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/linear_model/sgd_fast.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/metrics/cluster/expected_mutual_info_fast.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/metrics/pairwise_fast.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/neighbors/ball_tree.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/svm/liblinear.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/svm/libsvm.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/svm/libsvm_sparse.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/tree/_tree.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/utils/arraybuilder.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/utils/arrayfuncs.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/utils/graph_shortest_path.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/utils/lgamma.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/utils/murmurhash.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/utils/random.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/utils/seq_dataset.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/utils/sparsefuncs.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/utils/sparsetools/_csgraph.so
python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/utils/weight_vector.so
  See: https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Devel_Packages

Marcelo Barbosa (firemanxbr)

Spec URL: http://guaix.fis.ucm.es/~spr/python-scikit-learn.spec
SRPM URL: http://guaix.fis.ucm.es/~spr/python-scikit-learn-0.13.1-3.fc19.src.rpm

(In reply to Marcelo Barbosa from comment #10)
> Sergio,
> 
>   I'm reviewing your package, please check some issues:
> 
> - Large documentation must go in a -doc subpackage.
>   Note: Documentation size is 2222080 bytes in 280 files.
>   See:
> http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation
> 

I have removed documentation (for the moment), html build fails with sphinx and our our current docutils

> - Setting Variables with a only one request for line , example:
>   BuildRequires: python2-devel 
>   BuildRequires: numpy 
>   See:
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#BuildRequires

Done

> 
> - Review your licence, i believe mix licences in this package. I don't see a
> BSD license at all.
> 
>    "License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses found:
>      "BSD (3 clause)", "BSD (2 clause)", "Unknown or generated". 492 files
>      have unknown license. Detailed output of licensecheck in
>      /home/marcelo.barbosa/rpmbuild/SOURCES/reviews/976714-python-scikit-
>      learn/licensecheck.txt"
> 
>   See:
> http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
>   See:
> https://fedoraproject.org/wiki/Packaging:
> LicensingGuidelines#Multiple_Licensing_Scenarios 
> 

No License file is shipped, but the web page says "BSD 3 clauses"

The different licenses are both BSD (either 3 clauses or 2 clauses) as per https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses
so the total license is BSD and BSD == BSD

I have added a comment below License in the specfile

> - Package consistently uses macro is (instead of hard-coded directory names).
>   See:
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#Macros
> 

Where is this?


> - Please see devel files unversioned:
> Unversioned so-files
> --------------------
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/__check_build/_check_build.so
> python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/_hmmc.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/cluster/_hierarchical.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/cluster/_k_means.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/datasets/_svmlight_format.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/ensemble/_gradient_boosting.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/feature_extraction/_hashing.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/linear_model/cd_fast.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/linear_model/sgd_fast.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/metrics/cluster/
> expected_mutual_info_fast.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/metrics/pairwise_fast.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/neighbors/ball_tree.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/svm/liblinear.so
> python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/svm/libsvm.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/svm/libsvm_sparse.so
> python-scikit-learn: /usr/lib64/python2.7/site-packages/sklearn/tree/_tree.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/utils/arraybuilder.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/utils/arrayfuncs.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/utils/graph_shortest_path.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/utils/lgamma.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/utils/murmurhash.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/utils/random.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/utils/seq_dataset.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/utils/sparsefuncs.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/utils/sparsetools/_csgraph.so
> python-scikit-learn:
> /usr/lib64/python2.7/site-packages/sklearn/utils/weight_vector.so
>   See:
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#Devel_Packages
> 

These are not .so files of a devel package, they are Python extensions, that happen to be dynamic libraries ending with .so 


> Marcelo Barbosa (firemanxbr)

Hi Sergio,

- Output rpmlint is false positive

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[-]: Package does not contain kernel modules.
[-]: Package contains no static executables.
[-]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[-]: Package is not known to require ExcludeArch.
[x]: Package complies to the Packaging Guidelines
[x]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "BSD (3 clause)", "BSD (2 clause)", "Unknown or generated". 492 files
     have unknown license. Detailed output of licensecheck in
     /home/marcelo.barbosa/rpmbuild/SOURCES/reviews/976714-python-scikit-
     learn/licensecheck.txt
[x]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[-]: Useful -debuginfo package or justification otherwise.
[x]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 604160 bytes in 168 files.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Fully versioned dependency in subpackages, if present.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).

Python:
[x]: Python eggs must not download any dependencies during the build process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define.

===== EXTRA items =====

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: python-scikit-learn-0.13.1-3.fc19.x86_64.rpm
python-scikit-learn.x86_64: W: spelling-error %description -l en_US matplotlib -> diplomatic
1 packages and 0 specfiles checked; 0 errors, 1 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint python-scikit-learn
python-scikit-learn.x86_64: W: spelling-error %description -l en_US matplotlib -> diplomatic
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
# echo 'rpmlint-done:'



Requires
--------
python-scikit-learn (rpmlib, GLIBC filtered):
    libatlas.so.3()(64bit)
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libm.so.6()(64bit)
    libptcblas.so.3()(64bit)
    libptf77blas.so.3()(64bit)
    libpthread.so.0()(64bit)
    libpython2.7.so.1.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    numpy
    python(abi)
    rtld(GNU_HASH)
    scipy



Provides
--------
python-scikit-learn:
    python-scikit-learn
    python-scikit-learn(x86-64)


Source checksums
----------------
https://pypi.python.org/packages/source/s/scikit-learn/scikit-learn-0.13.1.tar.gz :
  CHECKSUM(SHA256) this package     : a6e4759a779ba792435d096c882a0d66ee29d369755c09209f1a4e50877bdc94
  CHECKSUM(SHA256) upstream package : a6e4759a779ba792435d096c882a0d66ee29d369755c09209f1a4e50877bdc94


Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
Buildroot used: fedora-19-x86_64
Command line :/usr/bin/fedora-review -b 976714

----------------

PACKAGE APPROVED

----------------

Marcelo Barbosa - (firemanxbr)

New Package SCM Request
=======================
Package Name: python-scikit-learn
Short Description: Machine learning in Python
Owners: sergiopr
Branches: f18 f19
InitialCC:

Git done (by process-git-requests).

Thank you everyone! It is built in rawhide, f18 and f19, so I'm closing

Didn't anybody notice the lots of bundled libs in there during review?

This mustn't have been approved, because it bundles:

* libsvm
* liblinear
* cblas from atlas
* blas (converted from Fortran to C using f2c)
* python-six (in new upstream version 0.14.X)
* python-joblib

I think here needs to be some work done, update to new upstream release and a new review is needed.

Cheers,
  Björn

Some more thoughts on this:

This is working on Python3.  Why hasn't a Python3-pkg been build?
There is some possibility to build some beautiful html-docs.  Why didn't you package them, too?

For there-review, I'll take this to make sure we'll bring this to high-quality.

Björn,

(In reply to Björn Esser from comment #17)
> 
> For there-review, I'll take this to make sure we'll bring this to
> high-quality.

thanks for reopening and taking this!

Package Change Request
======================
Package Name: python-scikit-learn
InitialCC: ml-sig


Please add new InitialCC to all existing branches.  Thanks a lot in advance!

Done.