Handle pathlib.Path path types

[cjacoby]

Since python 3.4 there has been a new object-oriented interface to handling paths. This PR adds support for pathlib.Path for functions which take a path as input. I make heavy use of pathlib.Path in my code these days, and while I can do the cast to string myself, I find that more and more libraries are supporting pathlib, and so I offer this PR.

In practice, since pathlib.Path is implicitly cast to string, there are only a couple of minor places where this is important - any function which performs a string-based operation on a variable which could be a path.

In order to make sure the tests pass, because this a python3-only feature, I also removed 2.7 from your .travis.yml file. Since python2 is EOL, and your readme doesn't include a python2 badge in the top, it seemed like this should be okay. If not, let me know and I'll see if I can come up with some test flags for python2 which would skip these tests or something.

[coveralls]

Coverage increased (+0.006%) to 99.109% when pulling 3aca341870f0be02e41a75e83fee582ef72347d8 on cjacoby:pathlib-paths into f657b23ffee6353cec8f63518bcc0f1e6b5ece6b on rabitt:master.

[cjacoby]

Looks like support for pathlib in os was added in python 3.6.

Q: Do you intend to support python < 3.6? If so, I can add the necessary modifications, otherwise I think it would be much simpler to say ">=3.6". I haven't found any evidence from poking around the repo about what you intend to support. For comparable reference though, numpy currently supports >3.5 but only runs their builds on >3.7 (https://github.com/numpy/numpy/blob/master/tox.ini), pandas >=3.6.1. I think given other similar movements in the ecosystem that it would be reasonable to support >=3.6.

[rabitt]

Thanks for the PR!

In order to make sure the tests pass, because this a python3-only feature, I also removed 2.7 from your .travis.yml file. Since python2 is EOL, and your readme doesn't include a python2 badge in the top, it seemed like this should be okay. If not, let me know and I'll see if I can come up with some test flags for python2 which would skip these tests or something.

Yes no problem. At this point we don't need to support 2.7 anymore

Q: Do you intend to support python < 3.6? If so, I can add the necessary modifications, otherwise I think it would be much simpler to say ">=3.6". I haven't found any evidence from poking around the repo about what you intend to support. For comparable reference though, numpy currently supports >3.5 but only runs their builds on >3.7 (https://github.com/numpy/numpy/blob/master/tox.ini), pandas >=3.6.1. I think given other similar movements in the ecosystem that it would be reasonable to support >=3.6.

For now I'd like to continue to support python >= 3 but < 3.6. In particular 3.5 is unfortunately still important because it's the default python on google VMs, but hopefully that will change soon. Am I right in assuming that the adjustment needed to support < 3.6 would work for any python version between 3.0 and 3.6? What about adding a patch, but putting a future release deprecation warning?

[cjacoby]

Thanks for the PR!

In order to make sure the tests pass, because this a python3-only feature, I also removed 2.7 from your .travis.yml file. Since python2 is EOL, and your readme doesn't include a python2 badge in the top, it seemed like this should be okay. If not, let me know and I'll see if I can come up with some test flags for python2 which would skip these tests or something.

Yes no problem. At this point we don't need to support 2.7 anymore

Q: Do you intend to support python < 3.6? If so, I can add the necessary modifications, otherwise I think it would be much simpler to say ">=3.6". I haven't found any evidence from poking around the repo about what you intend to support. For comparable reference though, numpy currently supports >3.5 but only runs their builds on >3.7 (https://github.com/numpy/numpy/blob/master/tox.ini), pandas >=3.6.1. I think given other similar movements in the ecosystem that it would be reasonable to support >=3.6.

For now I'd like to continue to support python >= 3 but < 3.6. In particular 3.5 is unfortunately still important because it's the default python on google VMs, but hopefully that will change soon. Am I right in assuming that the adjustment needed to support < 3.6 would work for any python version between 3.0 and 3.6? What about adding a patch, but putting a future release deprecation warning?

Then I'd recommend supporting >=3.5 for now. Looking at https://endoflife.date/python, 3.5 is technically also EOL as of september. There were significant updates in python starting with 3.5, including but not limited to full support for type hinting / pep 484. So, if you were to say min=3.5, then you could could allow type hints in the code without causing trouble. [1]

[1] Also, support for async / coroutines. This might actually be useful for this library, since you're calling out to external subprocesses to talk to sox. https://docs.python.org/3/library/asyncio-subprocess.html. That could in theory allow for significant speed up.

[rabitt]

Then I'd recommend supporting >=3.5 for now. Looking at https://endoflife.date/python, 3.5 is technically also EOL as of september. There were significant updates in python starting with 3.5, including but not limited to full support for type hinting / pep 484. So, if you were to say min=3.5, then you could could allow type hints in the code without causing trouble. [1]

Works for me!

[hadware]

Ah! You beat me to it. I was actually currently adding type annotation everywhere to function signatures and planning on converting the os.path calls to the pathlib API. Should I stop @rabitt ? (the type annotation might break some older 3.x versions).

Edit: I think most of the type annotation stuff is supported by 3.5. Maybe there are some backports, i'll look into it. Also, i'm all for dropping 2.7.

[cjacoby]

If we set 3.5 as a minimum version, then changing all the os calls to pathlib is actually better, since the problem I'm experiencing here is that in 3.5 (fixed in 3.6), is that os doesn't know how to handle pathlibs unless you cast them to strings yourself.

I'm happy to do that os => pathlib as a part of this PR if you like, since that's a more elegant final solution, I think. I'll add some limited type annotations where it makes sense with these changes, but there it's way too big a job to do in this PR, methinks.

[rabitt]

If we set 3.5 as a minimum version, then changing all the os calls to pathlib is actually better, since the problem I'm experiencing here is that in 3.5 (fixed in 3.6), is that os doesn't know how to handle pathlibs unless you cast them to strings yourself.

I'm happy to do that os => pathlib as a part of this PR if you like, since that's a more elegant final solution, I think. I'll add some limited type annotations where it makes sense with these changes, but there it's way too big a job to do in this PR, methinks.

Works for me! Agree that extending the type annotations elsewhere warrants a second PR.

Ah! You beat me to it. I was actually currently adding type annotation everywhere to function signatures and planning on converting the os.path calls to the pathlib API. Should I stop @rabitt ? (the type annotation might break some older 3.x versions).

Edit: I think most of the type annotation stuff is supported by 3.5. Maybe there are some backports, i'll look into it. Also, i'm all for dropping 2.7.

@hadware As long as pysox works on 3.5 and up, I'm fine with it, and would love to have type annotations added to the library. Sounds like it might be worth waiting for the PR to merge - I'll do my best to get it reviewed ASAP once it's ready.

[cjacoby]

Alright - now that there's consensus/buy-in, I'll try to finish it by tomorrow night / Monday morning.

[hadware]

Great work @cjacoby . I've "synchronized" my branch (and future PR) with your updates.

@rabitt : my PR is almost ready, I'll just be waiting for this PR to close to make sure that i'm up-to-date.

[cjacoby]

@rabitt I'm done here, as far as I can tell. Let me know if you want any changes.