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

✨ Add filters to HfApi.get_repo_discussions #1845

Merged
merged 9 commits into from
Nov 22, 2023

Conversation

SBrandeis
Copy link
Contributor

TL;DR

Add new filters to the HfApi.get_repo_discussions method

This PR depends on https://github.com/huggingface/moon-landing/pull/8147 (private) for the author filter.

Ping @Narsil

How it works

import huggingface_hub

discussion_generator = huggingface_hub.get_repo_discussions(
    repo_id="openai/whisper-large-v3",
    author="sanchit-gandhi",
    discussion_type="pull_request",
    discussion_status="open",
)

for discussion in discussion_generator:
    print(discussion)
    # ^ Only open pull requests authored by `sanchit-gandhi`

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Nov 20, 2023

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

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.

Thanks for the direct PR @SBrandeis ❤️

Looks good to me. 2 things I would add:

  • mention in the docs that it's possible to filter by type, status and author. A small paragraph + a unique example with all 3 filters at once is enough (somewhere here).
  • add tests to check filter are correctly sent to the server. Something like the snippet below here.
def test_get_repo_discussion_by_type(self):
    discussions = list(self._api.get_repo_discussions(repo_id=self.repo_id, discussion_type="discussion"))
    self.assertEqual(len(discussions), 1)
    self.assertEqual(discussions[0].num, self.discussion.num)

def test_get_repo_discussion_by_author(self):
    discussions = list(self._api.get_repo_discussions(repo_id=self.repo_id, discussion_type=USER))
    self.assertEqual(len(discussions), 2)
    
    discussions = list(self._api.get_repo_discussions(repo_id=self.repo_id, discussion_type="julien-c"))
    self.assertEqual(len(discussions), 0)

src/huggingface_hub/hf_api.py Outdated Show resolved Hide resolved
Copy link
Member

@julien-c julien-c 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!

@SBrandeis SBrandeis requested a review from Wauplin November 22, 2023 13:52
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (a9e9b71) 48.88% compared to head (88d3708) 82.07%.
Report is 2 commits behind head on main.

❗ Current head 88d3708 differs from pull request most recent head c67fd92. Consider uploading reports for the commit c67fd92 to get more accurate results

Files Patch % Lines
src/huggingface_hub/hf_api.py 78.57% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1845       +/-   ##
===========================================
+ Coverage   48.88%   82.07%   +33.19%     
===========================================
  Files          64       64               
  Lines        7765     7783       +18     
===========================================
+ Hits         3796     6388     +2592     
+ Misses       3969     1395     -2574     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Thanks a lot for the changes @SBrandeis! Looks good to me :)

docs/source/en/guides/community.md Outdated Show resolved Hide resolved
Comment on lines +83 to +86
DiscussionTypeFilter = Literal["all", "discussion", "pull_request"]
DISCUSSION_TYPES: Tuple[DiscussionTypeFilter, ...] = typing.get_args(DiscussionTypeFilter)
DiscussionStatusFilter = Literal["all", "open", "closed"]
DISCUSSION_STATUS: Tuple[DiscussionTypeFilter, ...] = typing.get_args(DiscussionStatusFilter)
Copy link
Contributor

Choose a reason for hiding this comment

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

🤯

tests/test_hf_api.py Outdated Show resolved Hide resolved
@Wauplin Wauplin merged commit 1b1049a into huggingface:main Nov 22, 2023
11 of 14 checks passed
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.

4 participants