-
Notifications
You must be signed in to change notification settings - Fork 581
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
Fixed the issue #2535 Added user followers and following in class User and added test cases for it #2536
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick PR @Amrit02102004! Looks good to me except that you've been faster than the deployment process 😄 Let's wait 1-2 days before merging this just to be sure.
tests/test_hf_api.py
Outdated
assert isinstance(overview.num_following, (int, type(None))) | ||
assert overview.num_following is None or overview.num_following >= 0 | ||
assert isinstance(overview.num_followers, (int, type(None))) | ||
assert overview.num_followers is None or overview.num_followers >= 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert isinstance(overview.num_following, (int, type(None))) | |
assert overview.num_following is None or overview.num_following >= 0 | |
assert isinstance(overview.num_followers, (int, type(None))) | |
assert overview.num_followers is None or overview.num_followers >= 0 | |
assert overview.num_following > 300 | |
assert overview.num_followers > 1000 |
We want to have a meaningful test. In our case, julien-c
has many following and followers so it's preferable to test that. I realized the API is not yet released (should be the case in 1-2 days), that's why you are currently getting None
. Let's wait a bit and we'll merge once the API is proded.
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Thanks for the quick review😊 |
The feature has been proded so let's merge this. Thanks again @Amrit02102004! 🤗 |
Welcome I am also happy to contribute my first contribution here |
Issue : #2535
Added the variable in docs also for future reference
Fix #2535.