search

imageio / imageio-ffmpeg

FFMPEG wrapper for Python
BSD 2-Clause "Simplified" License
225 stars 50 forks source link

Migrate to pathlib? #6

Closed paulmueller closed 5 years ago

paulmueller commented 5 years ago

Since imageio-ffmpeg requires Python>=3.4, I would suggest migrating to pathlib. I could create a PR for that.

almarklein commented 5 years ago

I wonder if it would actually reduce the amount of code; there are not a lot of paths. I've never really become a fan of pathlib because of the common need to turn it into a str. I would certainly be up for supporting pathlib objects in the main api functions though. Looking forward to your suggestions!

paulmueller commented 5 years ago

I guess it's a personal preference. I like pathlib, because I think it is easier to read the code. But I agree that in _utils.py, pathlib would not make a huge difference.

almarklein commented 5 years ago

Do you think it would be worth checking if path is a pathlib object in the functions in io.py?

paulmueller commented 5 years ago

Yes, it would definitely be worth doing path = pathlib.Path(path) at the beginning of each function and then consequently working with path.open, path.exists(), etc.

I currently don't have time for this right now.

Note: If you want to do this for imageio as well, you could support Python2 by putting "pathlib;python_version<='3.4'" in setup_requires in setup.py. The pathlib library on PyPI does not support all functionalities of the Python3 standard library (such as Path.glob I think), so you might run into issues during testing. I think there is also a pathlib2 library or somesuch.

almarklein commented 5 years ago

Yes, it would definitely be worth doing path = pathlib.Path(path) at the beginning of each function

Well, the path is more or less fed into subprocess.Popen without change, so that won't make sense. And its not always an actual path, but e.g. a specifier for a camera. What I meant was whether we should make it possible for users to use a pathlib object. Maybe it can be as simple as doing path = str(path).

If you want to do this for imageio as well, you could support Python2 by putting ...

Or wait a few months, so we can stop supporting Python 2 altogether :)

paulmueller commented 5 years ago

But str(path) does not always work for Python2 (unicode decode errors...). So waiting is probably the best solution anyway :smiley:.

almarklein commented 5 years ago

Ah, but imageio-ffmpeg is py3k only.

paulmueller commented 5 years ago

Yes, sorry. The str(path)-python2-problem would only apply to imageio.