Add interface to `fsspec` #943

mariosasko · 2022-07-07T13:35:54Z

Adds an interface to fsspec to enable access to the Hub as if it were a local file system.

This implementation uses the following scheme to infer parameters required for the protocol class initialization from a remote URL:
hf://{repo_id}[@{revision}]:/path_in_repo

For instance, this means one can simply push a Pandas dataframe to the Hub as follows:

df.to_csv("hf://repo_id:/path_in_repo", storage_options={"repo_type": "dataset"})

Things to discuss

if open's mode is "w", the current implementation doesn't create a repo to store the file if it doesn't already exist and throws an error instead. Let me know if you think it should create a repo.
also, feel free to suggest a better protocol scheme

Notes

this PR builds upon datasets' HfFileSystem (that version doesn't expose methods that can modify a Hub repo)
(for local testing) currently requires calling fsspec.register_implementation(HfFileSystem.protocol, HfFileSystem) to register the hf protocol in the local registry -> as soon as this PR is merged I'll add the protocol to fsspec's "official" registry to make this step unnecesarry

TODOs

add docs in a subsequent PR
add the hf protocol to fsspec's "official" registry

HuggingFaceDocBuilderDev · 2022-07-07T13:38:54Z

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

lhoestq

I love it !

Note that usually such a filesystem implementation is in its own package, e.g. s3fs, gcsfs, ossfs, adlfs. So if we want to be consistent with the rest of the ecosystem we could have hffs that requires huggingface_hub

cc @severo @julien-c this can be useful for datasets-server to use dask to process parquet files in hub repos 👀

lhoestq · 2022-07-07T13:47:51Z

src/huggingface_hub/hf_filesystem.py

+                # TODO(QL): add sizes
+                self._repo_entries_spec[hf_file.rfilename] = {
+                    "name": hf_file.rfilename,
+                    "size": None,


The file sizes are still not available in the siblings ?

Yep, still not available. Maybe @SBrandeis can help us with that.

mariosasko · 2022-07-07T15:05:52Z

@lhoestq Yes, I think having a dedicated package is also OK. huggingface_hub usually stores Hub integrations (and is well-established), so this is the reason why I put the code here. But as long as we register this implementation in the "official" registry, discoverability shouldn't be a problem anyways.

Still, I would like to hear what others think.

lhoestq · 2022-07-20T13:33:40Z

cc @adrinjalali @osanseviero do you have an opinion about whether this class should be in huggingface_hub or in a separate package hffs ?

adrinjalali · 2022-07-20T14:19:58Z

If it's going to depend on huggingface_hub then I have no preference. But as a user, I'd expect a package like hffs to be or to depend on a package much smaller than hfh. So no strong preference here on my side.

lhoestq · 2022-07-20T15:27:27Z

Given that it requires an extra dependency fsspec and for consistency with the other filesystems contributed by the community, I'd be down to create the hffs package

julien-c · 2022-07-20T15:52:39Z

no strong opinion either

Given that it's not a lot of code I think having it here would be fine, for simplicity purposes.

Does fsspec bring other transitive dependencies, or is it dependency-less?

lhoestq · 2022-07-20T16:15:36Z

Does fsspec bring other transitive dependencies, or is it dependency-less?

It has no dependencies

julien-c · 2022-07-20T16:18:03Z

what do you think @osanseviero @Pierrci @SBrandeis?

SBrandeis · 2022-07-21T08:21:06Z

Can we publish a "subpart" of huggingface_hub as a package on PyPI?
Maybe it makes sense to have two separate packages but to maintain them in the same repo (huggingface_hub)?
I don't know if this is doable or desirable.

adrinjalali · 2022-07-21T08:28:31Z

Can we publish a "subpart" of huggingface_hub as a package on PyPI?

I've also argued in favor of this in the past, but folks have made the argument that it might not get enough attention if it's a small library. I like the idea of a huggingface_hub-core which only includes the REST wrappers and probably the git interfaces. If that happens, we can also re-work the git interface part.

LysandreJik

No strong opinion either regarding hosting it in its own package. If that's the convention (as it seems to be with s3fs, gcsfs etc) then definitely fine for me to split it, but if you'd like to contribute it here for simplicity's sake and to offer a powerful API on top of it, it's also fine by me.

Do you expect a lot of changes to be done to this code, or do you expect it to be pretty static? Do you expect users to use this feature (or hffs) directly, or for it to be used under the hood in libraries such as datasets?

LysandreJik · 2022-07-21T09:05:37Z

setup.py

@@ -40,6 +40,7 @@ def get_version() -> str:
    "pytest-cov",
    "datasets",
    "soundfile",
+    "fsspec",


It should be added to its own extras so that it can be installed via pip install huggingface_hub[fsspec] or be added as a requirement for a downstream library requiring it

lhoestq · 2022-07-21T10:21:35Z

Do you expect users to use this feature (or hffs) directly, or for it to be used under the hood in libraries such as datasets?

We already have a similar class in datasets (read-only though) that we use to run glob patterns in dataset repositories.

I think the main usage is going to be

for pandas/dask users to read/write to HF Hub (this also includes the datasets-server project, which is likely to use dask for distributed processing)
for HF users training models and streaming data from the Hub (via datasets or others like torch datapipes)
for HF power users who need to do advanced manipulations in repos

mariosasko · 2022-07-21T12:49:10Z

Even though the implementation is pretty simple at the moment, it can quickly become more complex if we decide to override some more fsspec methods, add support for async IO, etc., so I think it's OK to have a separate repo to avoid unnecessary clutter in this repo (I'm also not a fan of having multiple packages/submodules in this repo tbh) and to follow fsspec's convention.

osanseviero · 2022-07-21T13:10:46Z

Maybe it makes sense to have two separate packages but to maintain them in the same repo (huggingface_hub)?
I don't know if this is doable or desirable.

I would avoid two libraries in this same repo as this has led to confusion in the past + maintenance becomes a bit trickier.

I don't have a strong opinion between having the code within huggingface_hub library or if it should be consistent with rest of ecosystem hffs, but hffs makes a bit more sense I think.

mariosasko · 2022-07-25T15:57:39Z

@lhoestq I've made some changes (namely the use of self.dircache for caching) to be more consistent with the existing implementations. Let me know what you think.

lhoestq · 2022-07-27T16:56:27Z

created the repo here: https://github.com/huggingface/hffs

Wauplin · 2022-08-17T13:10:30Z

Hey everyone 👋 It seems you all came to the conclusion to continue the work in the separate repo https://github.com/huggingface/hffs. Does that mean we can close this PR then ?

Btw I really like the df.to_csv("hf://repo_id:/path_in_repo") syntax ! It looks super cool 🔥

mariosasko added 2 commits July 7, 2022 14:25

Add HfFileSystem

02cccad

Add tests

f4f53ea

mariosasko requested review from adrinjalali and lhoestq July 7, 2022 13:35

lhoestq reviewed Jul 7, 2022

View reviewed changes

Add append support

091dcce

Fix temp_file handling

7a22592

SBrandeis mentioned this pull request Jul 21, 2022

✨ Add files_metadata option to repo_info #951

Merged

LysandreJik reviewed Jul 21, 2022

View reviewed changes

Use dircache for caching

8bb5a98

mariosasko closed this Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add interface to `fsspec` #943

Add interface to `fsspec` #943

mariosasko commented Jul 7, 2022 •

edited

Loading

HuggingFaceDocBuilderDev commented Jul 7, 2022 •

edited

Loading

lhoestq left a comment •

edited

Loading

lhoestq Jul 7, 2022

mariosasko Jul 7, 2022

SBrandeis Jul 21, 2022

mariosasko commented Jul 7, 2022

lhoestq commented Jul 20, 2022

adrinjalali commented Jul 20, 2022

lhoestq commented Jul 20, 2022

julien-c commented Jul 20, 2022

lhoestq commented Jul 20, 2022

julien-c commented Jul 20, 2022

SBrandeis commented Jul 21, 2022

adrinjalali commented Jul 21, 2022

LysandreJik left a comment

LysandreJik Jul 21, 2022

lhoestq commented Jul 21, 2022

mariosasko commented Jul 21, 2022

osanseviero commented Jul 21, 2022

mariosasko commented Jul 25, 2022

lhoestq commented Jul 27, 2022

Wauplin commented Aug 17, 2022

Add interface to fsspec #943

Add interface to fsspec #943

Conversation

mariosasko commented Jul 7, 2022 • edited Loading

Things to discuss

Notes

TODOs

HuggingFaceDocBuilderDev commented Jul 7, 2022 • edited Loading

lhoestq left a comment • edited Loading

Choose a reason for hiding this comment

lhoestq Jul 7, 2022

Choose a reason for hiding this comment

mariosasko Jul 7, 2022

Choose a reason for hiding this comment

SBrandeis Jul 21, 2022

Choose a reason for hiding this comment

mariosasko commented Jul 7, 2022

lhoestq commented Jul 20, 2022

adrinjalali commented Jul 20, 2022

lhoestq commented Jul 20, 2022

julien-c commented Jul 20, 2022

lhoestq commented Jul 20, 2022

julien-c commented Jul 20, 2022

SBrandeis commented Jul 21, 2022

adrinjalali commented Jul 21, 2022

LysandreJik left a comment

Choose a reason for hiding this comment

LysandreJik Jul 21, 2022

Choose a reason for hiding this comment

lhoestq commented Jul 21, 2022

mariosasko commented Jul 21, 2022

osanseviero commented Jul 21, 2022

mariosasko commented Jul 25, 2022

lhoestq commented Jul 27, 2022

Wauplin commented Aug 17, 2022

Add interface to `fsspec` #943

Add interface to `fsspec` #943

mariosasko commented Jul 7, 2022 •

edited

Loading

HuggingFaceDocBuilderDev commented Jul 7, 2022 •

edited

Loading

lhoestq left a comment •

edited

Loading