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

test: cover more fsspec backends #1015

Merged
merged 31 commits into from
Nov 7, 2023
Merged

test: cover more fsspec backends #1015

merged 31 commits into from
Nov 7, 2023

Conversation

lobis
Copy link
Collaborator

@lobis lobis commented Nov 2, 2023

Add test for more fsspec backends: memory, zip, tar.

I also modifed the github uri test to run but skip if the server returns 403 error (too many requests) this way it's tested some of the time.

Probably nobody will ever use uproot to open .root.zip files but in any case it's a good test to check the parsing of complex uris.

From adding this tests I noticed that the uproot._util.file_object_path_split fails when chaining fsspec protocols on windows paths. I modified the function so it would work (and added additional tests).

@lobis lobis mentioned this pull request Nov 2, 2023
@lobis lobis changed the base branch from main to fsspec-ssh-paramiko November 3, 2023 14:49
Base automatically changed from fsspec-ssh-paramiko to main November 3, 2023 16:47
@lobis lobis requested a review from jpivarski November 7, 2023 22:15
@lobis lobis marked this pull request as ready for review November 7, 2023 22:16
Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also modifed the github uri test to run but skip if the server returns 403 error (too many requests) this way it's tested some of the time.

✔️

Probably nobody will ever use uproot to open .root.zip files but in any case it's a good test to check the parsing of complex uris.

Actually, people often zip their ROOT files to upload them to GitHub Issues, which rejects file extensions that it doesn't recognize. So this saves one step in testing!

From adding this tests I noticed that the uproot._util.file_object_path_split fails when chaining fsspec protocols on windows paths. I modified the function so it would work (and added additional tests).

✔️

The memory:// scheme could be used with memory-mapped files, replacing MemmapSource. Then again, I can't think of any reason why that's better than just using MemmapSource...

But okay!

This PR is ready to be merged, as soon as you're ready.

@lobis lobis merged commit 98b1d48 into main Nov 7, 2023
21 checks passed
@lobis lobis deleted the fsspec-tests branch November 7, 2023 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants