[Bug 1372345] New: Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

Bug ID: 1372345
           Summary: Review Request: python-piexif - Pure Python library to
                    simplify exif manipulations with python
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]

Spec URL: https://jamatos.fedorapeople.org/python-piexif.spec
SRPM URL: https://jamatos.fedorapeople.org/python-piexif-1.0.5-1.fc25.src.rpm
Description:
Very simple Python library to simplify exif manipulations that does
not depend on other libraries.

There are only just five functions:

load(filename)                 - Get exif data as dict.
    dump(exif_dict)                - Get exif as bytes to save with JPEG.
    insert(exif_bytes, filename)   - Insert exif into JPEG.
    remove(filename)               - Remove exif from JPEG.
    transplant(filename, filename) - Transplant exif from JPEG to JPEG.

Fedora Account System Username: jamatos

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

José Matos  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |1365723, 1353169

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=1353169
[Bug 1353169] Review Request: python-nikola - A static website and blog
generator
https://bugzilla.redhat.com/show_bug.cgi?id=1365723
[Bug 1365723] nikola build throws ImportError: No module named piexif

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

Igor Gnatenko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|[e-mail]    |[e-mail]

--- Comment #1 from Igor Gnatenko  ---
I will review it today/tomorrow.

Initial comments:
> BuildRequires:  python-setuptools

BuildRequires:  python2-setuptools
> %{python2_sitelib}/%{pypi_name}

%{python2_sitelib}/%{pypi_name}/
> %{python2_sitelib}/%{pypi_name}-%{version}-py?.?.egg-info

%{python2_sitelib}/%{pypi_name}-*.egg-info/
or
%{python2_sitelib}/%{pypi_name}-%{version}-py%{python2_version}.egg-info/
(note trailing slash)

.. same for python3

* I think it has missing Requires: pythonX-pillow
* I would not put tests out of tree, as if something changes on github you will
not notice.
* I would just build from git tag (and write comment that this commit is
actually version X.Y.Z) - https://github.com/hMatoba/Piexif/issues/23
* Don't duplicate %description, define it just once
* Move BuildRequires under subpackages for better look&feel

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

--- Comment #2 from José Matos  ---
(In reply to Igor Gnatenko from comment #1)
> I will review it today/tomorrow.

Thank you. :-)
> Initial comments:
> > BuildRequires:  python-setuptools
>
> BuildRequires:  python2-setuptools

Right, I used pyp2rpm to generate the first version and this detail was lost in
the revision. I will fix it in the next revision.
> > %{python2_sitelib}/%{pypi_name}
>
> %{python2_sitelib}/%{pypi_name}/

Why do we need the trailing slash?

IIRC in %files

mydir

is the same as

%dir mydir
mydir/*

I admit that my memory could be failing. :-)
> > %{python2_sitelib}/%{pypi_name}-%{version}-py?.?.egg-info
>
> %{python2_sitelib}/%{pypi_name}-*.egg-info/
> or
> %{python2_sitelib}/%{pypi_name}-%{version}-py%{python2_version}.egg-info/
> (note trailing slash)
>
> .. same for python3

I will change to the * version or else it will fail at python 3.10. :-)
> I think it has missing Requires: pythonX-pillow

No it does not. I looked into the code, it mainly imports struct and io that
are in the standard library.

It is supposed to work with pillow and thus the tests but it does not require
it.
> I would not put tests out of tree, as if something changes on github you
>
> will not notice.
> I would just build from git tag (and write comment that this commit is
>
> actually version X.Y.Z) - https://github.com/hMatoba/Piexif/issues/23

I was expecting to have the nice upstream to release a new version soon. We can
always hope.
> Don't duplicate %description, define it just once

I saw a tip from you recently but I forgot it. How do you suggest to have a
single description?
> Move BuildRequires under subpackages for better look&feel

Fair request. :-)

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

--- Comment #3 from Igor Gnatenko  ---
(In reply to José Matos from comment #2)
> (In reply to Igor Gnatenko from comment #1)
> > I will review it today/tomorrow.
>
> Thank you. :-)
> > Initial comments:
> > > BuildRequires:  python-setuptools
> >
> > BuildRequires:  python2-setuptools
>
> Right, I used pyp2rpm to generate the first version and this detail was lost
> in the revision. I will fix it in the next revision.
> > > %{python2_sitelib}/%{pypi_name}
> >
> > %{python2_sitelib}/%{pypi_name}/
>
> Why do we need the trailing slash?

distutils creates file .egg-info, but setuptools creates directory .egg-info.
As you have BR: setuptools it's better to be sure that it's used ;)
> IIRC in %files
>
> mydir
>
> is the same as
>
> %dir mydir
> mydir/*
>
> I admit that my memory could be failing. :-)
> > > %{python2_sitelib}/%{pypi_name}-%{version}-py?.?.egg-info
> >
> > %{python2_sitelib}/%{pypi_name}-*.egg-info/
> > or
> > %{python2_sitelib}/%{pypi_name}-%{version}-py%{python2_version}.egg-info/
> > (note trailing slash)
> >
> > .. same for python3
>
> I will change to the * version or else it will fail at python 3.10. :-)

No, it will not ;) it's not regex, it's glob. * takes any numbers of any
characters.
> > I think it has missing Requires: pythonX-pillow
>
> No it does not. I looked into the code, it mainly imports struct and io that
> are in the standard library.
>
> It is supposed to work with pillow and thus the tests but it does not
> require it.

Ah, okay.
> > Don't duplicate %description, define it just once
>
> I saw a tip from you recently but I forgot it. How do you suggest to have a
> single description?

%global _description \
foo\
bar.
%description %{_description}
%description -n python2-%{modname} %{_description}

Python 2 version.

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

--- Comment #4 from José Matos  ---
Take 2. :-)

Now that upstream released a new version with the tests on.

Spec URL: https://jamatos.fedorapeople.org/python-piexif.spec
SRPM URL: https://jamatos.fedorapeople.org/python-piexif-1.0.7-1.fc25.src.rpm

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

Dmitry Tantsur  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #5 from Dmitry Tantsur  ---
Any updates, folks? Sorry for being annoying, but python-nikola is half-broken
without this package in F24.

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

Igor Gnatenko  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |POST
              Flags|                            |fedora-review+

--- Comment #6 from Igor Gnatenko  ---
LGTM, but couple of things:

* New release 1.0.8 is out, so during the import please update
* I would recommend to use GitHub archive instead of PyPI archives
* If you still use PyPI, consider using tar.gz instead of zip

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

--- Comment #7 from Jon Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/rpms/python-piexif

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

--- Comment #8 from Fedora Update System  ---
python-piexif-1.0.8-1.fc24 has been submitted as an update to Fedora 24.
https://bodhi.fedoraproject.org/updates/FEDORA-2017-f816b494a7

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|POST                        |MODIFIED

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

--- Comment #9 from Fedora Update System  ---
python-piexif-1.0.8-1.fc25 has been submitted as an update to Fedora 25.
https://bodhi.fedoraproject.org/updates/FEDORA-2017-39f0b59f50

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |ON_QA

--- Comment #10 from Fedora Update System  ---
python-piexif-1.0.8-1.fc24 has been pushed to the Fedora 24 testing repository.
If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-f816b494a7

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

--- Comment #11 from Fedora Update System  ---
python-piexif-1.0.8-1.fc25 has been pushed to the Fedora 25 testing repository.
If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-39f0b59f50

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

--- Comment #12 from Fedora Update System  ---
python-piexif-1.0.10-1.fc24 has been submitted as an update to Fedora 24.
https://bodhi.fedoraproject.org/updates/FEDORA-2017-f816b494a7

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |MODIFIED

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

--- Comment #13 from Fedora Update System  ---
python-piexif-1.0.10-1.fc25 has been submitted as an update to Fedora 25.
https://bodhi.fedoraproject.org/updates/FEDORA-2017-39f0b59f50

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |ON_QA

--- Comment #14 from Fedora Update System  ---
python-piexif-1.0.10-1.fc24 has been pushed to the Fedora 24 testing
repository. If problems still persist, please make note of it in this bug
report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-f816b494a7

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

[Red Hat Bugzilla]

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

--- Comment #15 from Fedora Update System  ---
python-piexif-1.0.10-1.fc25 has been pushed to the Fedora 25 testing
repository. If problems still persist, please make note of it in this bug
report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here:
https://bodhi.fedoraproject.org/updates/FEDORA-2017-39f0b59f50

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

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

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
         Resolution|---                         |ERRATA
        Last Closed|                            |2017-02-06 19:22:05

--- Comment #16 from Fedora Update System  ---
python-piexif-1.0.10-1.fc25 has been pushed to the Fedora 25 stable repository.
If problems still persist, please make note of it in this bug report.

[Bug 1372345] Review Request: python-piexif - Pure Python library to simplify exif manipulations with python

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

--- Comment #17 from Fedora Update System  ---
python-piexif-1.0.10-1.fc24 has been pushed to the Fedora 24 stable repository.
If problems still persist, please make note of it in this bug report.