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

Paginate repo likers endpoint #2530

Merged
merged 5 commits into from
Sep 11, 2024
Merged

Paginate repo likers endpoint #2530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d00f824
Paginate repo likers endpoint
hanouticelina Sep 10, 2024
7eec1c0
Merge branch 'main' into paginate-likers
hanouticelina Sep 10, 2024
9562d68
run in production
hanouticelina Sep 10, 2024
526b2d1
Remove unnecessary token usage in test_list_repo_likers
hanouticelina Sep 10, 2024
f56cc9a
Merge branch 'main' of github.com:huggingface/huggingface_hub into pa…
hanouticelina Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 4 additions & 18 deletions src/huggingface_hub/hf_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2342,7 +2342,7 @@ def list_repo_likers(
*,
repo_type: Optional[str] = None,
token: Union[bool, str, None] = None,
) -> List[User]:
) -> Iterable[User]:
"""
List all users who liked a given repo on the hugging Face Hub.

Expand All @@ -2364,29 +2364,15 @@ def list_repo_likers(
`None`.

Returns:
`List[User]`: a list of [`User`] objects.
`Iterable[User]`: an iterable of [`huggingface_hub.hf_api.User`] objects.
"""

# Construct the API endpoint
if repo_type is None:
repo_type = constants.REPO_TYPE_MODEL
path = f"{self.endpoint}/api/{repo_type}s/{repo_id}/likers"
headers = self._build_hf_headers(token=token)

# Make the request
response = get_session().get(path, headers=headers)
hf_raise_for_status(response)

# Parse the results into User objects
likers_data = response.json()
return [
User(
username=user_data["user"],
fullname=user_data["fullname"],
avatar_url=user_data["avatarUrl"],
)
for user_data in likers_data
]
for liker in paginate(path, params={}, headers=self._build_hf_headers(token=token)):
yield User(username=liker["user"], fullname=liker["fullname"], avatar_url=liker["avatarUrl"])
Comment on lines +2374 to +2375
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this is unfortunately a breaking change since users cannot do list_repo_likers(...)[0] anymore. It might break some script in the wild. We already had a similar issue when we switched list_models (and datasets/spaces) to paginated results in the past. It was much more painful as it was used everywhere.

Here I'm not sure there is much adoption yet so I would be tempted to merge "as is" and break things rather than having a half-solution for months. @LysandreJik what's your take on this?

For context, I've checked https://github.com/search?q=list_repo_likers&type=code&p=1 and didn't find much third-party integrations.

Copy link
Member

Choose a reason for hiding this comment

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

I would tend to say that a deprecation cycle rarely hurts but if you're certain it's not being used much yet, then maybe breaking this one is fine

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is that a deprecation cycle on an output type is a bit more annoying. I tried something for list_models in the past (#1143) but we got quite some problems and misunderstanding from users... 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since there is not so much adoption (compared to list_models() or list_datasets()), imo it's worth merging as is rather than handling a deprecation cycle


@validate_hf_hub_args
def model_info(
Expand Down
35 changes: 10 additions & 25 deletions tests/test_hf_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
RepoUrl,
SpaceInfo,
SpaceRuntime,
User,
WebhookInfo,
WebhookWatchedItem,
repo_type_and_id_from_hf_id,
Expand All @@ -81,18 +82,9 @@
hf_raise_for_status,
logging,
)
from huggingface_hub.utils.endpoint_helpers import (
_is_emission_within_threshold,
)
from huggingface_hub.utils.endpoint_helpers import _is_emission_within_threshold

from .testing_constants import (
ENDPOINT_STAGING,
FULL_NAME,
OTHER_TOKEN,
OTHER_USER,
TOKEN,
USER,
)
from .testing_constants import ENDPOINT_STAGING, FULL_NAME, OTHER_TOKEN, OTHER_USER, TOKEN, USER
from .testing_utils import (
DUMMY_DATASET_ID,
DUMMY_DATASET_ID_REVISION_ONE_SPECIFIC_COMMIT,
Expand Down Expand Up @@ -2972,21 +2964,14 @@ def test_list_likes_repos_auth_and_explicit_user(self) -> None:
likes = self.api.list_liked_repos(user=OTHER_USER, token=TOKEN)
self.assertEqual(likes.user, OTHER_USER)

@with_production_testing
def test_list_repo_likers(self) -> None:
# Create a repo + like
repo_id = self.api.create_repo(repo_name(), token=TOKEN).repo_id
self.api.like(repo_id, token=TOKEN)

# Use list_repo_likers to get the list of users who liked this repo
likers = self.api.list_repo_likers(repo_id, token=TOKEN)

# Check if the test user is in the list of likers
liker_usernames = [user.username for user in likers]
self.assertGreater(len(likers), 0)
self.assertIn(USER, liker_usernames)

# Cleanup
self.api.delete_repo(repo_id, token=TOKEN)
# a repo with > 5000 likes
all_likers = list(
HfApi().list_repo_likers(repo_id="open-llm-leaderboard/open_llm_leaderboard", repo_type="space")
)
self.assertIsInstance(all_likers[0], User)
self.assertGreater(len(all_likers), 5000)

@with_production_testing
def test_list_likes_on_production(self) -> None:
Expand Down
Loading