Skip to content

Allow pathlib.Path to be passed to all engines #4701

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

Merged
merged 7 commits into from
Dec 18, 2020
Merged

Allow pathlib.Path to be passed to all engines #4701

merged 7 commits into from
Dec 18, 2020

Conversation

alexamici
Copy link
Collaborator

@alexamici alexamici commented Dec 16, 2020

Once ZarrStore.open_group and NetCDF4DataStore.open accepts pathlib.Path objects we can remove _normalize_path from apiv2.open_dataset. All other backends accept them.

Note that zarr itself doesn't support them, yet. See: zarr-developers/zarr-python#601

@max-sixty
Copy link
Collaborator

LGTM @alexamici !

Could we add a small test / param onto an existing test? Or do you want to handle that as part of the larger PRs?

@alexamici
Copy link
Collaborator Author

@max-sixty there's no direct test for the backend Store classes, only indirect tests through open_dataset. For example all other beckends do support pathlib.Path but none is tested.

Once we remove _normalize_path (PR is coming, and we may need a bit of discussion before approval) we will be able to test all backends.

I propose to merge this PR as it is.

@alexamici alexamici requested a review from aurghs December 18, 2020 10:14
@alexamici alexamici marked this pull request as ready for review December 18, 2020 13:00
@alexamici alexamici requested a review from aurghs December 18, 2020 13:00
@alexamici alexamici merged commit 7355c35 into pydata:master Dec 18, 2020
@max-sixty
Copy link
Collaborator

Great! Thanks!

@aurghs aurghs mentioned this pull request Apr 1, 2021
@clbarnes
Copy link

clbarnes commented Sep 1, 2021

Zarr's convenience methods accept Path objects as of 2.9.0: https://zarr.readthedocs.io/en/stable/release.html#enhancements

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants