-
Notifications
You must be signed in to change notification settings - Fork 29
Pathlib for nixpy #550
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
Pathlib for nixpy #550
Conversation
|
thanks @wendtalexander. I guess I would like to have the tests included into the existing |
Depends what distro versions we're still targeting and if they have new enough h5py versions, I suppose. Is nixpy still available in any distribution repositories? |
|
@achilleas-k I browsed a bit and having a dependency > 2.0 does not seem to be a big deal even in Debian buster the version is 2.8.something. I guess it should not be a problem. |
|
Sounds like it should be fine then! |
nixio/file.py
Outdated
|
|
||
| import pathlib | ||
| import gc | ||
| from typing import Union |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit nit-picky, but would you mind to organize the imports a bit differently, i.e. the from xyz style import should be below the simple import xyz statements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the way isort handles import ordering. We could add that as a linter and enforce it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that would be nice to enforce this!
Currently, I am utilizing the pathlib module for all file system interactions. In my opinion, Nix should also adopt this approach. Instead of requiring manual conversion of paths to the appropriate encoding, Nix should accept path objects directly.
I have attempted to implement this change for you and provided a test demonstrating how to open a Nix file using a pathlib.Path object.
Furthermore, I have removed the encoding check for UTF-8 because h5py can handle emojis in path-like objects. However, I acknowledge that this change may impact backwards compatibility.
As of h5py 2.0.0, Unicode is supported for file names and objects in the file. When object names are read, they are returned as Unicode by default. For more information, please refer to the h5pyFAQ at h5pyFAQ.