Add support for path-like objects #924

PaperBag42 · 2019-03-06T19:23:55Z

If this is a bugfix, I have provided code that clearly demonstrates the problem and that works when used with this PR
I have added a test to the test suite, if necessary
I have properly documented new or changed features in the documention, or the docstrings
I have properly documented unusual changes to the code in the comments around it
I have made note of any breaking/backwards incompatible changes

Path-like objects are a new type of objects which are meant to represent file system paths. The concept was introduced in Python 3.4 as the pathlib.Path class, and later expanded to a full object protocol in Python 3.6. It is already supported by imageio, and I have been using it heavily myself.

I added a function that converts a path-like object to a regular string to the compat module, and used it in each method I found in the docs which requires a file name. I hope I didn't miss any.

Overdrivr · 2019-03-28T10:42:31Z

Thanks for the contribution !
Could you please add test cases for the different modified classes to ensure the fix works as expected ?

…any metadata (Zulko#926) * Add failing test * Fix Zulko#926 with fix from bertwagner

…o#921) Invalid video URL in docs/getting_started/compositing

PaperBag42 · 2019-03-31T18:22:39Z

@Overdrivr I added a test case in the test_PR module, I hope it's good.
I also think I messed up while rebasing my branch, I hope it's not too big of a deal 😅

Overdrivr · 2019-04-01T15:06:39Z

Thanks for the update, I'm not certain what happened when you rebased.
Could you merge Zulko:master into your branch to see if it fixes it ?

PaperBag42 · 2019-04-06T18:58:33Z

Sorry about the mess, but I think it's good to go now.
AppVeyor still fails, but this time it's because the latest ImageMagick version is missing from the server.

Overdrivr · 2019-04-08T05:49:41Z

@PaperBag42 Thanks for the heads-up, it's been fixed on master, can you try updating your branch ?

PaperBag42 · 2019-04-08T10:46:42Z

@Overdrivr Done, the tests are passing!

Overdrivr

Hi, I've made a few comments, thanks again for your contribution. I haven't personnally used pathlib2 so feel free to let me know if something doesn't make sense to you.

Overdrivr · 2019-04-08T12:32:33Z

tests/test_PR.py

+try:
+    from pathlib import Path
+except ImportError:
+    Path = str


There is a backport of Python3 pathlib for python 2. Could you remove this (line 16 to 19) and add pathlib2 as dependency ? This way your code will be active even on Python2

Overdrivr · 2019-04-08T12:32:56Z

tests/test_PR.py

+except ImportError:
+    Path = str
+
+sys.path.append("tests")


Please remove this, it should not be needed

Overdrivr · 2019-04-08T12:34:04Z

moviepy/compat.py

+
+        def fspath(path):             # Python 3.4-3.5
+            if isinstance(path, (string_types, pathlib.Path)):
+                return str(path)


Please see comment below regarding using pathlib2 instead of bypassing

PaperBag42 · 2019-04-11T18:02:37Z

I hadn't heard about pathlib2 before, good to know it exists.
I've made the changes, looks like it's working. (AppVeyor fails this time around because one of the tests took a microsecond too long to complete...)

Overdrivr · 2019-04-12T05:52:18Z

Thanks for the changes. I've noticed this test, it's randomly failing (I've tracked this issue with #943 and will try to fix it when I have some free time).

Some final feedback:

Could you split your test into multiple tests, so that if one fails it does not affect the others ?
Could you document this new feature into documentation ?

PaperBag42 · 2019-10-07T09:25:39Z

Sorry for disappearing for so long!
Anyway, I split the test function into smaller functions, and it looks like I had already added documentation in 17e7748. Have I missed a detail?

coveralls · 2020-02-23T07:39:41Z

Coverage increased (+0.3%) to 65.256% when pulling b80348d on PaperBag42:feature/pathlike-support into a16a107 on Zulko:master.

mgaitan · 2020-03-17T02:46:43Z

moviepy/compat.py

+
+try:
+    from os import fspath             # Python 3.6+
+except ImportError:


We agreed to implement #1081, so this compat function won't be necessary.

mgaitan · 2020-03-17T02:48:53Z

tests/test_PR.py

+    Test support for path-like objects as arguments for AudioFileClip.
+    """
+    with AudioFileClip(Path('media/crunching.mp3')) as audio:
+        audio.write_audiofile(Path(os.path.join(TMP_DIR, 'pathlike.mp3')))


Suggested change

audio.write_audiofile(Path(os.path.join(TMP_DIR, 'pathlike.mp3')))

audio.write_audiofile(Path(TMP_DIR) / 'pathlike.mp3')

mgaitan · 2020-03-17T02:49:31Z

tests/test_PR.py

+    Test support for path-like objects as arguments for VideoFileClip.
+    """
+    with VideoFileClip(Path('media/big_buck_bunny_432_433.webm')) as video:
+        video.write_videofile(Path(os.path.join(TMP_DIR, 'pathlike.mp4')))


Suggested change

video.write_videofile(Path(os.path.join(TMP_DIR, 'pathlike.mp4')))

video.write_videofile(Path(TMP_DIR) / 'pathlike.mp4')

tburrows13 · 2020-04-06T23:01:29Z

Hey @PaperBag42. You'd done everything necessary to get this merged at the time, but it never was. I tried to get this into a mergeable state today, but the merging process was a giant mess that I wasn't confident that I'd done it correctly (partially because we've reformatted the entire codebase (#1097) since this was written). We've also dropped support for Python 2, so all the compatibility stuff is now unnecessary.

I also thought that this would be a good place to instead use a decorator to automatically and consistently apply os.fspath() to the correct parameters. Given that all these combine to make some quite large changes, I thought it easier to create a new PR from scratch: #1137. Big thank you for doing the bulk of the work tracing down each place fspath() was needed and writing the tests, but, assuming that #1137 is merged, this PR can be closed.

PaperBag42 · 2020-04-07T23:54:49Z

Wow, I'm glad I was able to help after all!
Since the day I had opened this PR I moved on to other projects (and learned how to do a git rebase correctly). But at the time, Moviepy was of great help to me. So thank you for your work on this package!

PaperBag42 added 4 commits March 6, 2019 11:48

add fspath function to compat module

765accb

use fspath in functions and methods which require file names

c2179a2

accept path-likes, file-likes and bytes to imread in ImageClip

996c603

update documentation

17e7748

Overdrivr added the tests-needed PRs/code submissions which need test cases added to them. label Mar 28, 2019

tnoff and others added 7 commits March 31, 2019 19:27

Fix formatting in logger

c04ce1b

Fix ffmpeg_parse_infos silently hangs on Windows when file contains m…

54f7585

…any metadata (Zulko#926) * Add failing test * Fix Zulko#926 with fix from bertwagner

Do not install tests in site-packages (Zulko#880)

28eb665

Fix invalid video URL in document (getting_started/compositing) (Zulk…

116e2be

…o#921) Invalid video URL in docs/getting_started/compositing

add a test function

5391052

fix test for python 2.7

4e70506

another fix

67c6e72

PaperBag42 added 3 commits April 6, 2019 19:40

merge Zluko:master

4c46a87

Merge branch 'master' into feature/pathlike-support

bbbf12b

final test fix

f6c2352

merge branch 'master' into feature/pathlike-support

9078aaa

Overdrivr reviewed Apr 8, 2019

View reviewed changes

use pathlib2 for older python versions

fe9fbc4

Overdrivr added the documentation-needed Docs & docstrings which need adding or updating to rectify missing, incorrect or misleading info. label Apr 12, 2019

split test function to smaller tests

c893625

Merge branch 'master' into feature/pathlike-support

b80348d

tburrows13 mentioned this pull request Feb 23, 2020

Moviepy Roadmap - version 2.0 #1089

Open

7 tasks

tburrows13 added this to the Release v2.0.0 milestone Feb 24, 2020

tburrows13 added the feature New addition to the API i.e. a new class, method or parameter. label Mar 10, 2020

mgaitan reviewed Mar 17, 2020

View reviewed changes

tburrows13 mentioned this pull request Apr 6, 2020

Add support for path-like objects with a decorator #1137

Merged

4 tasks

PaperBag42 closed this Apr 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for path-like objects #924

Add support for path-like objects #924

PaperBag42 commented Mar 6, 2019

Overdrivr commented Mar 28, 2019

PaperBag42 commented Mar 31, 2019

Overdrivr commented Apr 1, 2019

PaperBag42 commented Apr 6, 2019

Overdrivr commented Apr 8, 2019

PaperBag42 commented Apr 8, 2019

Overdrivr left a comment

Overdrivr Apr 8, 2019

Overdrivr Apr 8, 2019

Overdrivr Apr 8, 2019

PaperBag42 commented Apr 11, 2019

Overdrivr commented Apr 12, 2019

PaperBag42 commented Oct 7, 2019

coveralls commented Feb 23, 2020

mgaitan Mar 17, 2020

mgaitan Mar 17, 2020

mgaitan Mar 17, 2020

tburrows13 commented Apr 6, 2020

PaperBag42 commented Apr 7, 2020

	audio.write_audiofile(Path(os.path.join(TMP_DIR, 'pathlike.mp3')))
	audio.write_audiofile(Path(TMP_DIR) / 'pathlike.mp3')

	video.write_videofile(Path(os.path.join(TMP_DIR, 'pathlike.mp4')))
	video.write_videofile(Path(TMP_DIR) / 'pathlike.mp4')

Add support for path-like objects #924

Add support for path-like objects #924

Conversation

PaperBag42 commented Mar 6, 2019

Overdrivr commented Mar 28, 2019

PaperBag42 commented Mar 31, 2019

Overdrivr commented Apr 1, 2019

PaperBag42 commented Apr 6, 2019

Overdrivr commented Apr 8, 2019

PaperBag42 commented Apr 8, 2019

Overdrivr left a comment

Choose a reason for hiding this comment

Overdrivr Apr 8, 2019

Choose a reason for hiding this comment

Overdrivr Apr 8, 2019

Choose a reason for hiding this comment

Overdrivr Apr 8, 2019

Choose a reason for hiding this comment

PaperBag42 commented Apr 11, 2019

Overdrivr commented Apr 12, 2019

PaperBag42 commented Oct 7, 2019

coveralls commented Feb 23, 2020

mgaitan Mar 17, 2020

Choose a reason for hiding this comment

mgaitan Mar 17, 2020

Choose a reason for hiding this comment

mgaitan Mar 17, 2020

Choose a reason for hiding this comment

tburrows13 commented Apr 6, 2020

PaperBag42 commented Apr 7, 2020