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.

Sign up for GitHub

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

Jump to bottom

Faster HfFileSystem.glob #1815

Merged
merged 24 commits into from
Nov 13, 2023
Merged

Faster HfFileSystem.glob #1815

merged 24 commits into from
Nov 13, 2023

Conversation

lhoestq
Copy link
Member

@lhoestq lhoestq commented Nov 10, 2023 •
edited
Loading

Following #1809

I added the expand_infoargument to _ls_tree instead of detail (it always returns dicts anyway) so that glob can still pass detail=True but with expand_info=False.

I also replaced the detail argument in info() with this one, since info() always return a dict anyway (which is consistent with detail=True).

(also let me know if you prefer naming the new argument expand instead of expand_info for consistency with the Hub API)

mariosasko and others added 17 commits November 3, 2023 16:00
@mariosasko
Remove HfFileSystem.exists override
440158d
@mariosasko
Add list_repo_tree to the API
c1fb3d8
@mariosasko
Rename BlobLastCommitInfo to LastCommitInfo
7a4a659
@mariosasko
Merge branch 'main' of github.com:huggingface/huggingface_hub into op…
49b7099 
…timize-hffs-find
@mariosasko
Add get_paths_info to the API
d0a3c8f
@mariosasko
Improve HfFileSystem tests
2c18e26
@mariosasko
Merge branch 'main' of github.com:huggingface/huggingface_hub into op…
b71b5e9 
…timize-hffs-find
@mariosasko
Faster HfFileSystem.find
c0cd365
@mariosasko
Tests
7602d24
@mariosasko
Merge branch 'main' of github.com:huggingface/huggingface_hub into fa…
8943634 
…ster-hffs-find
@mariosasko @Wauplin
Update src/huggingface_hub/hf_api.py
fd1b752 
Co-authored-by: Lucain <lucainp@gmail.com>
@mariosasko
Address the rest of comments
e23dbbc
@mariosasko
Fix comment
452b21e
@lhoestq
don't expand info for glob
d6fb0e3
@lhoestq
replace detail by expand_info in info()
7af1dfe
@lhoestq
remove usuned var
4b14b8e
@lhoestq
remove detail in _ls_tree
9a53b8f
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Nov 10, 2023 •
edited
Loading

The documentation is not available anymore as the PR was closed or merged.

@lhoestq lhoestq mentioned this pull request Nov 10, 2023
Merged
Base automatically changed from faster-hffs-find to main November 10, 2023 14:23
@mariosasko mariosasko mentioned this pull request Nov 10, 2023
Merged
mariosasko
mariosasko reviewed Nov 10, 2023
View reviewed changes
Copy link
Contributor

@mariosasko mariosasko left a comment

Choose a reason for hiding this comment

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

expand_info is more descriptive, so I slightly prefer it over expand

src/huggingface_hub/hf_file_system.py Outdated Show resolved Hide resolved
@lhoestq lhoestq marked this pull request as ready for review November 10, 2023 17:25
@lhoestq lhoestq marked this pull request as draft November 10, 2023 17:27
@lhoestq lhoestq marked this pull request as ready for review November 10, 2023 18:06
mariosasko
mariosasko approved these changes Nov 10, 2023
View reviewed changes
Copy link
Contributor

@mariosasko mariosasko left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

Wauplin
Wauplin approved these changes Nov 13, 2023
View reviewed changes
Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Looks good to me! So this PR is essentially to not fetch details by default in glob right? All the rest is only renaming variables?

@lhoestq
Copy link
Member Author

lhoestq commented Nov 13, 2023

Yes exactly :) I think datasets will use glob(..., detail=True, expand_info=False)

@lhoestq lhoestq merged commit acf036d into main Nov 13, 2023
10 of 14 checks passed
@lhoestq lhoestq deleted the faster-hffs-glob branch November 13, 2023 15:01
This was referenced Nov 15, 2023
Merged
Closed
@pmrowla pmrowla mentioned this pull request Feb 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mariosasko mariosasko mariosasko approved these changes

@Wauplin Wauplin Wauplin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lhoestq @HuggingFaceDocBuilderDev @Wauplin @mariosasko