Spec URL: https://guaix.fis.ucm.es/~spr/fedora/python-wcsaxes.spec SRPM URL: https://guaix.fis.ucm.es/~spr/fedora/python-wcsaxes-0.9-1.fc24.src.rpm Description: WCSAxes is a framework for making plots of Astronomical data in Matplotlib. Fedora Account System Username: sergiopr
Hi Sergio, the spec file looks good, but I have a couple of suggestions you might consider: * Please use the %{srcname} macro you have defined in the `SourceO` tag: > Source0: https://pypi.io/packages/source/w/wcsaxes/wcsaxes-%{version}.tar.gz change to `Source0: https://pypi.io/packages/source/w/%{srcname}/%{srcname}-%{version}.tar.gz` * To improve readability please use multiple lines for `BuildRequires` tag: > BuildRequires: python2-devel python3-devel xorg-x11-server-Xvfb * You can use macro %{summary}, which will contain content of the Summary tag (it's generated automatically, you do not have to define it). Just keep the first Summary tag as it is and in python2/3- subpackages you can use `Summary: %{summary}`. * It is better to avoid using wildcards in the %files section to have an idea what is installed with the package. Doing the following change would make it more safe (same for python3- subpackage): > %{python2_sitelib}/* change to %{python2_sitelib}/%{srcname} %{python2_sitelib}/%{srcname}-%{version}-py?.?.egg-info * Due to the above, the following file gets to the generated RPMs. It is generated during tests and does not belong in the package: /usr/lib/python3.5/site-packages/test.png /usr/lib/python2.7/site-packages/test.png ---- Another issue is that I believe you are running tests in %{buildroot} to avoid the problem with pytest-mpl dependency (https://github.com/astrofrog/wcsaxes/commit/83fec6e82bbae751f0241da6933ae425c0e9664b#diff-8b1c3fd0d4a6765c16dfd18509182f9dR8) and ignore wcsaxes/setup.cfg settings. However, when installing the package and running tests with wcsaxes.test() (as it is stated in the documentation), I am running into multiple errors: import file mismatch: imported module 'wcsaxes.tests.test_coordinate_helpers' has this __file__ attribute: /builddir/build/BUILDROOT/python-wcsaxes-0.9-1.fc24.x86_64/usr/lib/python3.5/site-packages/wcsaxes/tests/test_coordinate_helpers.py which is not the same as the test file we want to collect: /usr/lib/python3.5/site-packages/wcsaxes/tests/test_coordinate_helpers.py HINT: remove __pycache__ / .pyc files and/or use a unique basename for your test file modules That happens because running tests in the %{buildroot} generates files, e.g. /usr/lib/python3.5/site-packages/test.png (mentioned above). Instead of changing the directory, try specifying another config when running tests (with a -c option) or ignoring it, to fix the above issues. Another option would be to clean up after tests. Apart from a couple of small suggestions above, everything looks fine. Note, that it is an informal package review, as I am not sponsored yet, but I can do a formal one once I am.
Thanks for the review Iryna, wcsaxes.test() definitively doesn't work with this spec file
Hi Sergio, Is there still an interest in adding this package to Fedora? Let me know if you need any help.
I'm considering to package pytest-mpl before this one
Hi Sergio, python-wcsaxes is already in Fedora. *** This bug has been marked as a duplicate of bug 1395273 ***
Great, thank you for the review anyway