[Bug 1389695] Review Request: python-wcsaxes - A Python framework for plotting astronomical and geospatial data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1389695

Iryna Shcherbina <ishcherb@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ishcherb@xxxxxxxxxx



--- Comment #1 from Iryna Shcherbina <ishcherb@xxxxxxxxxx> ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]