search

mdshw5 / pyfaidx

Efficient pythonic random access to fasta subsequences
https://pypi.python.org/pypi/pyfaidx
Other
449 stars 75 forks source link

Using a pathlib.Path object as filename #183

Closed palao closed 2 years ago

palao commented 2 years ago

Hi, First off thanks a lot for pyfaidx. I like it very much. Now my question: Would it be a good idea in your opinion that pyfaidx.Fasta, pyfaidx.Faidx et al accept an instance of pathlib.Path instead of only accepting a string-like filename? I think it would be more pythonic; at least it was for me a surprise to get an AttributeError when I tried to create an instance of Fasta with a Path instead of a string. If you think it would be a good idea I can make a pull request, if you want.

mdshw5 commented 2 years ago

Hey @palao. Thanks for the suggestion. Since the pathlib module was new in python 3.4 I haven't bothered to add support. As long as support can be added in a way that degrades gracefully for python <3.4 I see no issue with including this feature and would welcome a PR.

mdshw5 commented 2 years ago

After looking at my test environment and realizing that it's become even more difficult to test against anything less than py3.6 I think you can add a dependency on pathlib as long as users can continue to pass strings using the existing interface.

palao commented 2 years ago

I was thinking in a simpler approach. I'll propose a PR and you tell me. I understand that you want to keep compatibility with python<3.4 for the production code. Is it the same for the tests?

mdshw5 commented 2 years ago

I believe it would be helpful to maintain compatibility with python<3.4 for production code (although production environments should really be pinning a package version) although I will no longer be testing under anything less than python3.6+. So at some point I expect it will be possible to clean up some of the compatibility shims that have accumulated over the years.

ap-- commented 2 years ago

This is supported in v0.7.0 via:

https://github.com/mdshw5/pyfaidx/blob/58d8b4e381f1348b9e669d55d88019e2498c71de/pyfaidx/__init__.py#L367-L368

and

https://github.com/mdshw5/pyfaidx/blob/58d8b4e381f1348b9e669d55d88019e2498c71de/pyfaidx/__init__.py#L382-L383

both of which are basically just checking via hasattr if the provided objects are instances of os.PathLike in a way that is backwards compatible with Python < 3.6