unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#44275] [PATCH] gnu: Add python-pydub.
@ 2020-10-28  8:56 Tanguy Le Carrour
  2020-10-28 16:34 ` Leo Famulari
  2020-10-29  9:09 ` [bug#44275] [PATCH v2] " Tanguy Le Carrour
  0 siblings, 2 replies; 11+ messages in thread
From: Tanguy Le Carrour @ 2020-10-28  8:56 UTC (permalink / raw)
  To: 44275; +Cc: Tanguy Le Carrour

* gnu/packages/python-xyz.scm (python-pydub): New variable.
---
 gnu/packages/python-xyz.scm | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index 6c5ccac647..55580a251b 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -16659,6 +16659,28 @@ ignoring formatting changes.")
 (define-public python2-pydiff
   (package-with-python2 python-pydiff))
 
+(define-public python-pydub
+  (package
+    (name "python-pydub")
+    (version "0.24.1")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (pypi-uri "pydub" version))
+       (sha256
+        (base32
+         "0sfwfq7yjv4bl3yqbmizszscafvwf4zr40hzbsy7rclvzyznh333"))))
+    (build-system python-build-system)
+    (home-page "http://pydub.com")
+    (propagated-inputs
+     `(("ffmpeg" ,ffmpeg)
+       ("python-scipy" ,python-scipy)))
+    (synopsis "Manipulate audio with an simple and easy high level interface")
+    (description
+     "@code{pydub} makes it easy to manipulate audio.  It relies on
+@code{ffmpeg} to open various audio formats.")
+    (license license:expat))) ; MIT license
+
 (define-public python-tqdm
   (package
     (name "python-tqdm")
-- 
2.29.1





^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#44275] [PATCH] gnu: Add python-pydub.
  2020-10-28  8:56 [bug#44275] [PATCH] gnu: Add python-pydub Tanguy Le Carrour
@ 2020-10-28 16:34 ` Leo Famulari
  2020-10-29  8:26   ` Tanguy Le Carrour
  2020-10-29  9:09 ` [bug#44275] [PATCH v2] " Tanguy Le Carrour
  1 sibling, 1 reply; 11+ messages in thread
From: Leo Famulari @ 2020-10-28 16:34 UTC (permalink / raw)
  To: Tanguy Le Carrour; +Cc: 44275

On Wed, Oct 28, 2020 at 09:56:54AM +0100, Tanguy Le Carrour wrote:
> * gnu/packages/python-xyz.scm (python-pydub): New variable.

> +    (propagated-inputs
> +     `(("ffmpeg" ,ffmpeg)
> +       ("python-scipy" ,python-scipy)))

It would be good if store references to these programs could be "baked in" to
the pydub code, rather than propagated. Can you take a look at how they
are called and see if that is feasible? Often there is only one location
that must be patched.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#44275] [PATCH] gnu: Add python-pydub.
  2020-10-28 16:34 ` Leo Famulari
@ 2020-10-29  8:26   ` Tanguy Le Carrour
  2020-10-29 14:14     ` Leo Famulari
  0 siblings, 1 reply; 11+ messages in thread
From: Tanguy Le Carrour @ 2020-10-29  8:26 UTC (permalink / raw)
  To: Leo Famulari; +Cc: 44275

Hi Leo,

Thanks for taking the time to review this!

Le 10/28, Leo Famulari a écrit :
> On Wed, Oct 28, 2020 at 09:56:54AM +0100, Tanguy Le Carrour wrote:
> > * gnu/packages/python-xyz.scm (python-pydub): New variable.
> 
> > +    (propagated-inputs
> > +     `(("ffmpeg" ,ffmpeg)
> > +       ("python-scipy" ,python-scipy)))
> 
> It would be good if store references to these programs could be "baked in" to
> the pydub code, rather than propagated. Can you take a look at how they
> are called and see if that is feasible? Often there is only one location
> that must be patched.

mmm… haven't done that so far, it would be my first time! Always good to
learn new tricks! :-)
I'll try to patch the call to `ffmpeg` and let you know.

`python-scipy` is a different problem, though. This module is not listed
as a requirement, but… it's required at run time. Event if I don't see the
point of having SciPy loaded when you want to manipulate audio. But that's
a different matter!

Regards,

-- 
Tanguy




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#44275] [PATCH v2] gnu: Add python-pydub.
  2020-10-28  8:56 [bug#44275] [PATCH] gnu: Add python-pydub Tanguy Le Carrour
  2020-10-28 16:34 ` Leo Famulari
@ 2020-10-29  9:09 ` Tanguy Le Carrour
  2020-10-30 20:40   ` bug#44275: " Leo Famulari
  1 sibling, 1 reply; 11+ messages in thread
From: Tanguy Le Carrour @ 2020-10-29  9:09 UTC (permalink / raw)
  To: 44275; +Cc: Tanguy Le Carrour, leo

* gnu/packages/python-xyz.scm (python-pydub): New variable.
---
 gnu/packages/python-xyz.scm | 38 +++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index 9f689d35b5..e938081d28 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -16683,6 +16683,44 @@ ignoring formatting changes.")
 (define-public python2-pydiff
   (package-with-python2 python-pydiff))
 
+(define-public python-pydub
+  (package
+    (name "python-pydub")
+    (version "0.24.1")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (pypi-uri "pydub" version))
+       (sha256
+        (base32
+         "0sfwfq7yjv4bl3yqbmizszscafvwf4zr40hzbsy7rclvzyznh333"))))
+    (build-system python-build-system)
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'fix-ffmpeg-path
+           (lambda* (#:key inputs #:allow-other-keys)
+             (let ((ffmpeg (assoc-ref inputs "ffmpeg")))
+               (substitute* '("pydub/utils.py")
+                 (("return \"ffmpeg\"")
+                  (string-append "return \"" ffmpeg "/bin/ffmpeg\""))
+                 (("return \"ffplay\"")
+                  (string-append "return \"" ffmpeg "/bin/ffplay\""))
+                 (("return \"ffprobe\"")
+                  (string-append "return \"" ffmpeg "/bin/ffprobe\""))
+                 (("warn\\(\"Couldn't find ff") "# warn\\(\"Couldn't find ff"))
+               #t))))))
+    (home-page "http://pydub.com")
+    (inputs
+     `(("ffmpeg" ,ffmpeg)))
+    (propagated-inputs
+     `(("python-scipy" ,python-scipy)))
+    (synopsis "Manipulate audio with an simple and easy high level interface")
+    (description
+     "@code{pydub} makes it easy to manipulate audio.  It relies on
+@code{ffmpeg} to open various audio formats.")
+    (license license:expat))) ; MIT license
+
 (define-public python-tqdm
   (package
     (name "python-tqdm")
-- 
2.29.1





^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#44275] [PATCH] gnu: Add python-pydub.
  2020-10-29  8:26   ` Tanguy Le Carrour
@ 2020-10-29 14:14     ` Leo Famulari
  2020-10-29 14:39       ` Tanguy Le Carrour
  2020-10-29 21:50       ` Leo Famulari
  0 siblings, 2 replies; 11+ messages in thread
From: Leo Famulari @ 2020-10-29 14:14 UTC (permalink / raw)
  To: Tanguy Le Carrour; +Cc: 44275

On Thu, Oct 29, 2020 at 09:26:57AM +0100, Tanguy Le Carrour wrote:
> Hi Leo,
> 
> Thanks for taking the time to review this!
> 
> Le 10/28, Leo Famulari a écrit :
> > On Wed, Oct 28, 2020 at 09:56:54AM +0100, Tanguy Le Carrour wrote:
> > > * gnu/packages/python-xyz.scm (python-pydub): New variable.
> > 
> > > +    (propagated-inputs
> > > +     `(("ffmpeg" ,ffmpeg)
> > > +       ("python-scipy" ,python-scipy)))
> > 
> > It would be good if store references to these programs could be "baked in" to
> > the pydub code, rather than propagated. Can you take a look at how they
> > are called and see if that is feasible? Often there is only one location
> > that must be patched.
> 
> mmm… haven't done that so far, it would be my first time! Always good to
> learn new tricks! :-)
> I'll try to patch the call to `ffmpeg` and let you know.
> 
> `python-scipy` is a different problem, though. This module is not listed
> as a requirement, but… it's required at run time. Event if I don't see the
> point of having SciPy loaded when you want to manipulate audio. But that's
> a different matter!

Okay, I will take a look at the scipy thing today.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#44275] [PATCH] gnu: Add python-pydub.
  2020-10-29 14:14     ` Leo Famulari
@ 2020-10-29 14:39       ` Tanguy Le Carrour
  2020-10-29 21:50       ` Leo Famulari
  1 sibling, 0 replies; 11+ messages in thread
From: Tanguy Le Carrour @ 2020-10-29 14:39 UTC (permalink / raw)
  To: Leo Famulari; +Cc: 44275

Le 10/29, Leo Famulari a écrit :
> On Thu, Oct 29, 2020 at 09:26:57AM +0100, Tanguy Le Carrour wrote:
> > […]
> > > It would be good if store references to these programs could be "baked in" to
> > > the pydub code, rather than propagated. Can you take a look at how they
> > > are called and see if that is feasible? Often there is only one location
> > > that must be patched.
> > […]
> > `python-scipy` is a different problem, though. This module is not listed
> > as a requirement, but… it's required at run time. Event if I don't see the
> > point of having SciPy loaded when you want to manipulate audio. But that's
> > a different matter!
> 
> Okay, I will take a look at the scipy thing today.

Great, thanks!

If it was up to me, I would remove altogether everything that is
related to `scipy`, meaning the `pydub/scipy_effects.py` file. But it
wouldn't be the same software any more, would it? ^_^'

Regards,

-- 
Tanguy




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#44275] [PATCH] gnu: Add python-pydub.
  2020-10-29 14:14     ` Leo Famulari
  2020-10-29 14:39       ` Tanguy Le Carrour
@ 2020-10-29 21:50       ` Leo Famulari
  2020-10-30  9:19         ` Tanguy Le Carrour
  1 sibling, 1 reply; 11+ messages in thread
From: Leo Famulari @ 2020-10-29 21:50 UTC (permalink / raw)
  To: Tanguy Le Carrour; +Cc: 44275

[-- Attachment #1: Type: text/plain, Size: 2175 bytes --]

On Thu, Oct 29, 2020 at 10:14:27AM -0400, Leo Famulari wrote:
> Okay, I will take a look at the scipy thing today.

I decided to let scipy be propagated since it's normal for Python things
to be propagated. But I still think we should hard-code the reference to
ffmpeg.

I looked at the code, and it finds ffmpeg-related programs in
'pydub/utils.py', in the functions get_encoder_name(),
get_player_name(), and get_prober_name().

I think it should be sufficient to substitute any mention of the words
"ffmpeg", "ffplay", and "ffprobe" with the full store-path of those
programs.

I included a diff on your patch. You can see exactly what it does by
adding (error "Stopping...") after the substitute*, building with
--keep-failed, and then looking at the 'pydub/utils.py' file.

diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index 3c2d882003e..47ec542e6d1 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -16696,9 +16696,24 @@ ignoring formatting changes.")
          "0sfwfq7yjv4bl3yqbmizszscafvwf4zr40hzbsy7rclvzyznh333"))))
     (build-system python-build-system)
     (home-page "http://pydub.com")
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'patch-ffmpeg-references
+           (lambda* (#:key inputs #:allow-other-keys)
+             (let* ((ffmpeg-store-item (assoc-ref inputs "ffmpeg"))
+                    (ffmpeg (string-append ffmpeg-store-item "/bin/ffmpeg"))
+                    (ffplay (string-append ffmpeg-store-item "/bin/ffplay"))
+                    (ffprobe (string-append ffmpeg-store-item "/bin/ffprobe")))
+               (substitute* "pydub/utils.py"
+                 (("ffmpeg") ffmpeg)
+                 (("ffplay") ffplay)
+                 (("ffprobe") ffprobe))
+               #t))))))
+    (inputs
+     `(("ffmpeg" ,ffmpeg)))
     (propagated-inputs
-     `(("ffmpeg" ,ffmpeg)
-       ("python-scipy" ,python-scipy)))
+     `(("python-scipy" ,python-scipy)))
     (synopsis "Manipulate audio with an simple and easy high level interface")
     (description
      "@code{pydub} makes it easy to manipulate audio.  It relies on

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [bug#44275] [PATCH] gnu: Add python-pydub.
  2020-10-29 21:50       ` Leo Famulari
@ 2020-10-30  9:19         ` Tanguy Le Carrour
  2020-10-30 20:42           ` Leo Famulari
  0 siblings, 1 reply; 11+ messages in thread
From: Tanguy Le Carrour @ 2020-10-30  9:19 UTC (permalink / raw)
  To: Leo Famulari; +Cc: 44275

Hi Leo!


Le 10/29, Leo Famulari a écrit :
> On Thu, Oct 29, 2020 at 10:14:27AM -0400, Leo Famulari wrote:
> > Okay, I will take a look at the scipy thing today.
> 
> I decided to let scipy be propagated since it's normal for Python things
> to be propagated. But I still think we should hard-code the reference to
> ffmpeg.
> 
> I looked at the code, and it finds ffmpeg-related programs in
> 'pydub/utils.py', in the functions get_encoder_name(),
> get_player_name(), and get_prober_name().
> 
> I think it should be sufficient to substitute any mention of the words
> "ffmpeg", "ffplay", and "ffprobe" with the full store-path of those
> programs.

+1…

> I included a diff on your patch. You can see exactly what it does by
> adding (error "Stopping...") after the substitute*, building with
> --keep-failed, and then looking at the 'pydub/utils.py' file.
> 
> diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
> index 3c2d882003e..47ec542e6d1 100644
> --- a/gnu/packages/python-xyz.scm
> +++ b/gnu/packages/python-xyz.scm
> @@ -16696,9 +16696,24 @@ ignoring formatting changes.")
>           "0sfwfq7yjv4bl3yqbmizszscafvwf4zr40hzbsy7rclvzyznh333"))))
>      (build-system python-build-system)
>      (home-page "http://pydub.com")
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'patch-ffmpeg-references
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (let* ((ffmpeg-store-item (assoc-ref inputs "ffmpeg"))
> +                    (ffmpeg (string-append ffmpeg-store-item "/bin/ffmpeg"))
> +                    (ffplay (string-append ffmpeg-store-item "/bin/ffplay"))
> +                    (ffprobe (string-append ffmpeg-store-item "/bin/ffprobe")))
> +               (substitute* "pydub/utils.py"
> +                 (("ffmpeg") ffmpeg)
> +                 (("ffplay") ffplay)
> +                 (("ffprobe") ffprobe))
> +               #t))))))
> +    (inputs
> +     `(("ffmpeg" ,ffmpeg)))
>      (propagated-inputs
> -     `(("ffmpeg" ,ffmpeg)
> -       ("python-scipy" ,python-scipy)))
> +     `(("python-scipy" ,python-scipy)))
>      (synopsis "Manipulate audio with an simple and easy high level interface")
>      (description
>       "@code{pydub} makes it easy to manipulate audio.  It relies on

So I guess you didn't see the one I submitted: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44275#14 :-(
I was really proud, because… it worked! :-)

But yours is shorter and does the job (even if it "brute-force" replaces all
the occurences of "ffmpeg", even in the comments!) and I'm totally fine
with it!

Thanks again for your help and your time!

-- 
Tanguy




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#44275: [PATCH v2] gnu: Add python-pydub.
  2020-10-29  9:09 ` [bug#44275] [PATCH v2] " Tanguy Le Carrour
@ 2020-10-30 20:40   ` Leo Famulari
  2020-10-30 21:10     ` [bug#44275] " Tanguy LE CARROUR
  0 siblings, 1 reply; 11+ messages in thread
From: Leo Famulari @ 2020-10-30 20:40 UTC (permalink / raw)
  To: Tanguy Le Carrour; +Cc: 44275-done

[-- Attachment #1: Type: text/plain, Size: 1376 bytes --]

On Thu, Oct 29, 2020 at 10:09:29AM +0100, Tanguy Le Carrour wrote:
> * gnu/packages/python-xyz.scm (python-pydub): New variable.

Thanks! Pushed as 96767739a1d2222ed802dd5dcfa2bda1df85df77

> +         (add-after 'unpack 'fix-ffmpeg-path
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (let ((ffmpeg (assoc-ref inputs "ffmpeg")))
> +               (substitute* '("pydub/utils.py")
> +                 (("return \"ffmpeg\"")
> +                  (string-append "return \"" ffmpeg "/bin/ffmpeg\""))
> +                 (("return \"ffplay\"")
> +                  (string-append "return \"" ffmpeg "/bin/ffplay\""))
> +                 (("return \"ffprobe\"")
> +                  (string-append "return \"" ffmpeg "/bin/ffprobe\""))
> +                 (("warn\\(\"Couldn't find ff") "# warn\\(\"Couldn't find ff"))
> +               #t))))))

This solution is more correct than the one I suggested. Thanks!

> +    (home-page "http://pydub.com")

I made this use HTTPS.

> +    (synopsis "Manipulate audio with an simple and easy high level interface")
> +    (description
> +     "@code{pydub} makes it easy to manipulate audio.  It relies on
> +@code{ffmpeg} to open various audio formats.")

And I tweaked these to avoid so-called "marketing language" and to
clarify that pydub is in Python, which I think can be useful when
searching for packages.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#44275] [PATCH] gnu: Add python-pydub.
  2020-10-30  9:19         ` Tanguy Le Carrour
@ 2020-10-30 20:42           ` Leo Famulari
  0 siblings, 0 replies; 11+ messages in thread
From: Leo Famulari @ 2020-10-30 20:42 UTC (permalink / raw)
  To: Tanguy Le Carrour; +Cc: 44275-done

On Fri, Oct 30, 2020 at 10:19:01AM +0100, Tanguy Le Carrour wrote:
> So I guess you didn't see the one I submitted: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44275#14 :-(

Oops! I missed your v2 patch by mistake.

> I was really proud, because… it worked! :-)

And it's better than my solution!




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [bug#44275] [PATCH v2] gnu: Add python-pydub.
  2020-10-30 20:40   ` bug#44275: " Leo Famulari
@ 2020-10-30 21:10     ` Tanguy LE CARROUR
  0 siblings, 0 replies; 11+ messages in thread
From: Tanguy LE CARROUR @ 2020-10-30 21:10 UTC (permalink / raw)
  To: Leo Famulari; +Cc: 44275-done

Hi,

Le 30 octobre 2020 21:40:47 CET, Leo Famulari <leo@famulari.name> a écrit :
>On Thu, Oct 29, 2020 at 10:09:29AM +0100, Tanguy Le Carrour wrote:
>> * gnu/packages/python-xyz.scm (python-pydub): New variable.
>
>Thanks! Pushed as 96767739a1d2222ed802dd5dcfa2bda1df85df77
>
>> +         (add-after 'unpack 'fix-ffmpeg-path
>> +           (lambda* (#:key inputs #:allow-other-keys)
>> +             (let ((ffmpeg (assoc-ref inputs "ffmpeg")))
>> +               (substitute* '("pydub/utils.py")
>> +                 (("return \"ffmpeg\"")
>> +                  (string-append "return \"" ffmpeg
>"/bin/ffmpeg\""))
>> +                 (("return \"ffplay\"")
>> +                  (string-append "return \"" ffmpeg
>"/bin/ffplay\""))
>> +                 (("return \"ffprobe\"")
>> +                  (string-append "return \"" ffmpeg
>"/bin/ffprobe\""))
>> +                 (("warn\\(\"Couldn't find ff") "# warn\\(\"Couldn't
>find ff"))
>> +               #t))))))
>
>This solution is more correct than the one I suggested. Thanks!
>
>> +    (home-page "http://pydub.com")
>
>I made this use HTTPS.
>
>> +    (synopsis "Manipulate audio with an simple and easy high level
>interface")
>> +    (description
>> +     "@code{pydub} makes it easy to manipulate audio.  It relies on
>> +@code{ffmpeg} to open various audio formats.")
>
>And I tweaked these to avoid so-called "marketing language" and to
>clarify that pydub is in Python, which I think can be useful when
>searching for packages.

Thanks!

-- 
Tanguy




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-10-30 21:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-28  8:56 [bug#44275] [PATCH] gnu: Add python-pydub Tanguy Le Carrour
2020-10-28 16:34 ` Leo Famulari
2020-10-29  8:26   ` Tanguy Le Carrour
2020-10-29 14:14     ` Leo Famulari
2020-10-29 14:39       ` Tanguy Le Carrour
2020-10-29 21:50       ` Leo Famulari
2020-10-30  9:19         ` Tanguy Le Carrour
2020-10-30 20:42           ` Leo Famulari
2020-10-29  9:09 ` [bug#44275] [PATCH v2] " Tanguy Le Carrour
2020-10-30 20:40   ` bug#44275: " Leo Famulari
2020-10-30 21:10     ` [bug#44275] " Tanguy LE CARROUR

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).