Bug 784847 - Review Request: APLpy - The Astronomical Plotting Library in Python
Summary: Review Request: APLpy - The Astronomical Plotting Library in Python
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Golo Fuchert
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-01-26 12:42 UTC by Germán Racca
Modified: 2012-04-30 13:28 UTC (History)
4 users (show)

Fixed In Version: APLpy-0.9.6-3.fc15
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-02-15 11:28:13 UTC
Type: ---
packages: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Germán Racca 2012-01-26 12:42:11 UTC
Spec: http://skytux.fedorapeople.org/packages/aplpy.spec

SRPM: http://skytux.fedorapeople.org/packages/aplpy-0.9.6-1.fc16.src.rpm

Description:

APLpy (the Astronomical Plotting Library in Python) is a Python module aimed at 
producing publication-quality plots of astronomical imaging data in FITS format.
The module uses Matplotlib, a powerful and interactive plotting package. It is
capable of creating output files in several graphical formats, including EPS,
PDF, PS, PNG, and SVG.

Main features:

o Make plots interactively or using scripts
o Show grayscale, colorscale, and 3-color RGB images of FITS files
o Generate co-aligned FITS cubes to make 3-color RGB images
o Overlay any number of contour sets
o Overlay markers with fully customizable symbols
o Plot customizable shapes like circles, ellipses, and rectangles
o Overlay ds9 region files
o Overlay coordinate grids
o Show colorbars, scalebars, and beams
o Customize the appearance of labels and ticks
o Hide, show, and remove different contour and marker layers
o Pan, zoom, and save any view as a full publication-quality plot
o Save plots as EPS, PDF, PS, PNG, and SVG 

________________________________________________________________________________

Koji builds from scratch:

F-15: http://koji.fedoraproject.org/koji/taskinfo?taskID=3734373
F-16: http://koji.fedoraproject.org/koji/taskinfo?taskID=3734348
F-17: http://koji.fedoraproject.org/koji/taskinfo?taskID=3734331
________________________________________________________________________________

rpmlint output:

Checking RPM package (aplpy-0.9.6-1.fc16.noarch.rpm)
--------------------
aplpy.noarch: E: explicit-lib-dependency python-matplotlib
aplpy.noarch: W: spelling-error %description -l en_US grayscale -> gray scale, gray-scale, graceless
aplpy.noarch: W: spelling-error %description -l en_US colorscale -> color scale, color-scale, colorless
aplpy.noarch: W: spelling-error %description -l en_US customizable -> customization
aplpy.noarch: W: spelling-error %description -l en_US colorbars -> color bars, color-bars, colors
aplpy.noarch: W: spelling-error %description -l en_US scalebars -> scale bars, scale-bars, scalars
aplpy.noarch: W: install-file-in-docs /usr/share/doc/aplpy-0.9.6/INSTALL
1 packages and 0 specfiles checked; 1 errors, 6 warnings.

Checking SPEC file (aplpy.spec)
------------------
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

Checking SRPM package (aplpy-0.9.6-1.fc16.src.rpm)
---------------------
aplpy.src: W: spelling-error %description -l en_US grayscale -> gray scale, gray-scale, graceless
aplpy.src: W: spelling-error %description -l en_US colorscale -> color scale, color-scale, colorless
aplpy.src: W: spelling-error %description -l en_US customizable -> customization
aplpy.src: W: spelling-error %description -l en_US colorbars -> color bars, color-bars, colors
aplpy.src: W: spelling-error %description -l en_US scalebars -> scale bars, scale-bars, scalars
1 packages and 0 specfiles checked; 0 errors, 5 warnings.

Done.

Comment 1 Golo Fuchert 2012-01-28 18:06:54 UTC
Hi Germán,

I will take this. The review shouldn't take long, since the package looks quite nice, but I have to investigate on that error reported by rpmlint (though it looks like a false positive), I assume you tried building the package without this?

Comment 2 Germán Racca 2012-01-30 11:44:01 UTC
Hi Golo:

Yes, I tried to build the package without the explicit requirement of matplotlib, but it didn't work:

$ rpm -qp --requires aplpy-0.9.6-1.fc16.noarch.rpm 
numpy  
pyfits  
python(abi) = 2.7
pywcs  
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PartialHardlinkSets) <= 4.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1

so there is no matplotlib in the dependencies of the package, that's why I had to make an explicit requirement of matplotlib.

Thanks for taking this :)

Comment 3 Sergio Pascual 2012-01-30 14:56:05 UTC
Hi, good to see you have packaged this, it was in my queue also.

Just a question about the name of the package. I happen to have packaged ATpy, (astronomical tables in python), another package by the same developers. 

In that case I chose to use the project name instead of the tarball name. Would you consider doing the same here for consistency? I remark this is only a suggestion, as the guidelines allow you to use the tarball name or the project name.


In any case, please add me as a co maintainer of package once it's reviewed

Comment 4 Germán Racca 2012-01-30 16:01:40 UTC
(In reply to comment #3)
> Hi, good to see you have packaged this, it was in my queue also.
> 
> Just a question about the name of the package. I happen to have packaged ATpy,
> (astronomical tables in python), another package by the same developers. 
> 
> In that case I chose to use the project name instead of the tarball name. Would
> you consider doing the same here for consistency? I remark this is only a
> suggestion, as the guidelines allow you to use the tarball name or the project
> name.
> 
> 
> In any case, please add me as a co maintainer of package once it's reviewed

Hi Sergio:

You mean APLpy instead of aplpy? Any problem for me, just waiting for the opinion of the reviewer. If it's all right, I can make the change right now.

Comment 5 Golo Fuchert 2012-01-30 21:00:25 UTC
Do it better now than after the review! ;)

Comment 6 Germán Racca 2012-01-31 00:16:30 UTC
Please find updated files here:

SPEC: http://skytux.fedorapeople.org/packages/APLpy.spec

SRPM: http://skytux.fedorapeople.org/packages/APLpy-0.9.6-2.fc16.src.rpm

Have a nice review :)

Comment 7 Golo Fuchert 2012-02-05 20:36:19 UTC
Hi Germán,

first of all, the bugzilla ticket needs to have the same name as the package itself before the SCM request, so you have to rename it to APLpy as well.

But now the review:

rpmlint SRPMS/APLpy-0.9.6-2.fc16.src.rpm RPMS/noarch/APLpy-0.9.6-2.fc16.noarch.rpm SPECS/APLpy.spec 
APLpy.src: W: spelling-error %description -l en_US grayscale -> gray scale, gray-scale, graceless
APLpy.src: W: spelling-error %description -l en_US colorscale -> color scale, color-scale, colorless
APLpy.src: W: spelling-error %description -l en_US customizable -> customization
APLpy.src: W: spelling-error %description -l en_US colorbars -> color bars, color-bars, colors
APLpy.src: W: spelling-error %description -l en_US scalebars -> scale bars, scale-bars, scalars
APLpy.noarch: E: explicit-lib-dependency python-matplotlib
APLpy.noarch: W: spelling-error %description -l en_US grayscale -> gray scale, gray-scale, graceless
APLpy.noarch: W: spelling-error %description -l en_US colorscale -> color scale, color-scale, colorless
APLpy.noarch: W: spelling-error %description -l en_US customizable -> customization
APLpy.noarch: W: spelling-error %description -l en_US colorbars -> color bars, color-bars, colors
APLpy.noarch: W: spelling-error %description -l en_US scalebars -> scale bars, scale-bars, scalars
APLpy.noarch: W: install-file-in-docs /usr/share/doc/APLpy-0.9.6/INSTALL
2 packages and 1 specfiles checked; 1 errors, 11 warnings.

All the warnings can be ignored, however the Packaging Guidelines recommend _not_ to contain INSTALL files. Concerning the error: This is a false positive. If you want to, you can report this as a bug (for rpmlint I guess). There is a filter for such packages.

[+] = ok
[o] = does not apply
[-] = not ok

MUST:

[+] The package is named according to the guidelines
[+] Spec file name matches base package name
[+] The package follows the Packaging Guidelines
[+] The license is an approved licence (MIT)
[+] The License field matches the actual licence
[+] License file from source file is included in %doc
[+] The spec file is written in American English
[+] The spec file is legible
[+] Used sources match with upstream sources (md5)

$ md5sum APLpy-0.9.6.tar.gz.*
bfd8e61ea1139dcc3d8bdf94eee03df3  APLpy-0.9.6.tar.gz.packaged
bfd8e61ea1139dcc3d8bdf94eee03df3  APLpy-0.9.6.tar.gz.upstream

[+] Package build at least on one primary architecture (i686)
[o] No architectures known, where the package doesn't build
[+] All build dependencies are listed in the BuildRequires section
[o] No locales for the package
[o] Package stores no shared libraries
[o] Package does not bundle copies of system libraries
[o] Package is not relocatable
[+] Package owns all directories it installs
[+] No files are listed more then once in the %files section
[+] File permissions are set properly (%defattr(...) is used)
[+] Consistent use of macros
[+] Package contains code and documentation only, no content
[+] No large documentation files
[+] %doc files do not affect runtime
[o] No Header files included
[o] No static libraries
[o] No library files ending with .so included
[o] No -devel subpackage
[+] No libtool .la archives included
[o] No GUI application, no need for a .desktop file
[+] Package does not own files or directories that are owned by other packages
[+] All filenames are valid UTF-8

SHOULD:

[+] The package builds in mock

python specific:
[+] Python egg is being built from source
[o] No compat package.

-----

Comments:

- You have to change the name of this ticket
- I would recommend not to include INSTALL in the package
- You may want to report a bug concerning the rpmlint error
- The spec file contains some tags or commands which are no
  longer needed if you package for Fedora only. Those that I noticed where:
  	BuildRoot
	%defattr(-,root,root,-)
	%clean
  Please consider removing them. If you intend to package for RHEL, some
  might still be needed, though.

----------------
Package APPROVED
----------------

Comment 8 Germán Racca 2012-02-06 12:52:10 UTC
(In reply to comment #7)
> Comments:
>
> - You have to change the name of this ticket
Changed

> - I would recommend not to include INSTALL in the package
Removed.

> - You may want to report a bug concerning the rpmlint error
I will do.

> - The spec file contains some tags or commands which are no
>   longer needed if you package for Fedora only. Those that I noticed where:
>    BuildRoot
>  %defattr(-,root,root,-)
>  %clean
>   Please consider removing them. If you intend to package for RHEL, some
>   might still be needed, though.
Removed.

> ----------------
> Package APPROVED
> ----------------

Thanks very much for your review! :)

Comment 9 Germán Racca 2012-02-06 13:00:09 UTC
New Package SCM Request
=======================
Package Name: APLpy
Short Description: The Astronomical Plotting Library in Python
Owners: skytux
Branches: f15 f16
InitialCC:

Comment 10 Gwyn Ciesla 2012-02-06 13:22:10 UTC
Git done (by process-git-requests).

Comment 11 Fedora Update System 2012-02-06 19:37:10 UTC
APLpy-0.9.6-3.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/APLpy-0.9.6-3.fc16

Comment 12 Fedora Update System 2012-02-06 19:54:20 UTC
APLpy-0.9.6-3.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/APLpy-0.9.6-3.fc15

Comment 13 Fedora Update System 2012-02-07 07:57:10 UTC
APLpy-0.9.6-3.fc16 has been pushed to the Fedora 16 testing repository.

Comment 14 Fedora Update System 2012-02-15 11:28:13 UTC
APLpy-0.9.6-3.fc16 has been pushed to the Fedora 16 stable repository.

Comment 15 Fedora Update System 2012-02-15 11:38:09 UTC
APLpy-0.9.6-3.fc15 has been pushed to the Fedora 15 stable repository.

Comment 16 Germán Racca 2012-04-24 12:45:21 UTC
Hi Golo:

There is a new version of this package, and there is a directory with tests, should I include it also? Should I run them in the spec file? Please could you take a look at the program and give me some help? Thanks!!

(version 0.9.8)
https://github.com/aplpy/aplpy/downloads

Comment 17 Golo Fuchert 2012-04-28 12:39:50 UTC
Hi Germán

the package guidelines for Fedora say nothing about tests, so I assume it should be safe to include them. However, I never had to deal with such a case before, so I don't know the answer. Thus, I would recommend to write to the devel mailinglist.
Here are some points you might want to consider:
 - I'm not sure what benefit the automated test in the spec file would
   have. What would you do in case of a negative outcome?
 - If the tests are included, is there a default location for this in Fedora?

As I said, I don't know the answers, but I'm pretty sure the mailinglist does.

Comment 18 Germán Racca 2012-04-30 13:28:27 UTC
(In reply to comment #17)
> Hi Germán
> 
> the package guidelines for Fedora say nothing about tests, so I assume it
> should be safe to include them. However, I never had to deal with such a case
> before, so I don't know the answer. Thus, I would recommend to write to the
> devel mailinglist.
> Here are some points you might want to consider:
>  - I'm not sure what benefit the automated test in the spec file would
>    have. What would you do in case of a negative outcome?
>  - If the tests are included, is there a default location for this in Fedora?
> 
> As I said, I don't know the answers, but I'm pretty sure the mailinglist does.

Thanks Golo for your answer. I'm going to ask the list.


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