Sebastian Schott <sschott@mailbox.org> writes:

> * gnu/packages/python-xyz.scm (python-pymediainfo): New variable.
> ---
>  gnu/packages/python-xyz.scm | 41 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
> index 2c308796e7..9f5cb20095 100644
> --- a/gnu/packages/python-xyz.scm
> +++ b/gnu/packages/python-xyz.scm
> @@ -166,6 +166,47 @@
>    #:use-module (srfi srfi-1)
>    #:use-module (srfi srfi-26))
>
> +(define-public python-pymediainfo
> +  (package
> +    (name "python-pymediainfo")
> +    (version "4.1")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (pypi-uri "pymediainfo" version))
> +       (sha256
> +        (base32
> +         "0mhpxs7vlqx8w75z93dy7nnvx89kwfdjkla03l19an15rlyqyspd"))))
> +    (build-system python-build-system)
> +    (native-inputs
> +     `(("python-setuptools-scm" ,python-setuptools-scm)
> +       ("python-pytest" ,python-pytest)
> +       ("python-pytest-runner" ,python-pytest-runner)))
> +    (inputs
> +     `(("libmediainfo" ,libmediainfo)))
> +    (arguments
> +     `(#:phases (modify-phases %standard-phases
> +                  (add-after 'unpack 'patch-libmediainfo
> +                    (lambda _
> +                      (substitute* "pymediainfo/__init__.py"
> +                        (("libmediainfo.so.0")
> +                         (string-append (assoc-ref %build-inputs "libmediainfo")
> +                                        "/lib/libmediainfo.so.0")))))

There's a convention of phases returning #t (I think), so it would be
good to just put #t at the end of this phase.

> +                  (replace 'check
> +                    (lambda _
> +                      ;; Extend PYTHONPATH so the built package will be found.
> +                      (setenv "PYTHONPATH"
> +                              (string-append (getcwd) "/build/lib:"
> +                                             (getenv "PYTHONPATH")))
> +                      (invoke "pytest" "-vv" "-k" "not
> test_parse_url"))))))

Here, is the "not test_parse_url" bit taken from the upstream source, or
does it have significance for the Guix package?

Also, if the only reason the check phase doesn't work is the PYTHONPATH,
you could always do something like:

  (add-before 'check 'set-PYTHONPATH
    (lambda _
      ;; Extend PYTHONPATH so the built package will be found.
      (setenv "PYTHONPATH"
              (string-append (getcwd) "/build/lib:"
                             (getenv "PYTHONPATH")))
      #t))

> +    (home-page
> +     "https://github.com/sbraz/pymediainfo")
> +    (synopsis
> +     "Python wrapper for the mediainfo library")
> +    (description
> +     "Python wrapper for the mediainfo library to access the
> technical and tag data for video and audio files.")

This line is a bit too long, not in terms of the value, but in terms of
how it's represented in the source code. Running guix lint on this
package should tell you this.

This is also true for some other new packages added in other patches in
this series, there's a list here [1].

1: https://guix-patches-data.cbaines.net/compare?base_commit=3cb8cb666069804551148b9c49ebaea6b3a9973e&target_commit=cf6f9c3d5d13c37af3852734778ec11b26ac8d4b

Thanks,

Chris