Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use pathlib vs os.path #87

Open
ebgoldstein opened this issue Feb 11, 2020 · 3 comments
Open

use pathlib vs os.path #87

ebgoldstein opened this issue Feb 11, 2020 · 3 comments
Labels
enhancement New feature or request

Comments

@ebgoldstein
Copy link
Contributor

ebgoldstein commented Feb 11, 2020

I think a lot of the Path manipulation can be done with the pathlib library that was introduced in Python 3.4. This might help clean up the code, rather than using os.path.

from @chrisleaman 's JOSS review: openjournals/joss-reviews#2075 (comment)

@ebgoldstein ebgoldstein added the enhancement New feature or request label Feb 11, 2020
@ebgoldstein
Copy link
Contributor Author

ebgoldstein commented Feb 11, 2020

@Matmorcat
Copy link
Contributor

Matmorcat commented Feb 13, 2020

@chrisleaman I'm not aware of any particular performance improvements with our code using a high level library for path manipulation over the os.path module other than enforcing pure paths. What were your thoughts as far as what it might improve?

@chrisleaman
Copy link
Contributor

From a performance perspective, I don't think there are any advantages of pathlib over os.path. But in terms of code and readability, I think pathlib can make your code a bit cleaner. I think one of the best things for this project would be the glob command, avoiding os.walk pattern and filtering out results:

from pathlib import Path

top_level_csv_files = Path.cwd().glob('*.csv')
all_csv_files = Path.cwd().rglob('*.csv')

I do realize it is a bit of a pain to go back and refactor everything as the package works perfectly fine as it is. Sorry if this came across as "you must use this package rather than that package" 😅! My suggestion was more changes/additions in the future and even for new other packages you might write 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants