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 object detection to inference client #1548

Merged

Conversation

dulayjm
Copy link
Contributor

@dulayjm dulayjm commented Jul 10, 2023

Add object-detection to HuggingFace🤗 Hub

References Issue #1539

This is an ongoing list of model tasks to implement in the Hugging Face Hub inference client. Each task is planned to be its own PR. The task for this is object-detection.

** Key Features **

  • Modifies _client.py to call an object detection model. The example model is DETR ResNet-50.
  • Modifies testing and documentation to reflect changes.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jul 10, 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.

Very clean implementation! Thanks for working on it @dulayjm !

I tested it locally and re-recorded the test (there were 3 requests made, making the file quite huge). Worked like a charm for me at first use :D If everything's fine in the CI, it is ready to be merged straight away 🤗

src/huggingface_hub/inference/_client.py Outdated Show resolved Hide resolved
src/huggingface_hub/inference/_generated/_async_client.py Outdated Show resolved Hide resolved
@dulayjm
Copy link
Contributor Author

dulayjm commented Jul 10, 2023

@Wauplin Thanks!

The code seems to still fail the CI, but I have a question about that. On the Python Quality check, it fails with some messages at the end. In particular, the LocalTokenNotFoundError is one I'm seeing a couple of times. Is there something that needs to be different about the VCR testing?

=========================== short test summary info ============================
FAILED ../tests/test_commit_scheduler.py::TestCommitScheduler::test_mocked_push_to_hub - AssertionError: 2 not greater than 2
FAILED ../tests/test_hf_api.py::HfApiListFilesInfoTest::test_list_files_with_expand - AssertionError: False is not true
FAILED ../tests/test_webhooks_server.py::TestWebhooksServerRun::test_run_parse_payload - AssertionError: 422 != 200
ERROR ../tests/test_hf_api.py::TestSpaceAPIProduction::test_manage_secrets - huggingface_hub.utils._headers.LocalTokenNotFoundError: Token is required (`token=True`), but no token found. You need to provide a token or be logged in to Hugging Face with `huggingface-cli login` or `huggingface_hub.login`. See https://huggingface.co/settings/tokens.
ERROR ../tests/test_hf_api.py::TestSpaceAPIProduction::test_pause_and_restart_space - huggingface_hub.utils._headers.LocalTokenNotFoundError: Token is required (`token=True`), but no token found. You need to provide a token or be logged in to Hugging Face with `huggingface-cli login` or `huggingface_hub.login`. See https://huggingface.co/settings/tokens.
ERROR ../tests/test_hf_api.py::TestSpaceAPIProduction::test_space_runtime - huggingface_hub.utils._headers.LocalTokenNotFoundError: Token is required (`token=True`), but no token found. You need to provide a token or be logged in to Hugging Face with `huggingface-cli login` or `huggingface_hub.login`. See https://huggingface.co/settings/tokens.
= 3 failed, 660 passed, 40 skipped, 60 deselected, 120 warnings, 3 errors in 342.14s (0:05:42) =
Error: Process completed with exit code 1.

@Wauplin
Copy link
Contributor

Wauplin commented Jul 10, 2023

@dulayjm All failing tests are unrelated. Some tests actually require a production token that is set as a secret on the repo. Since your PR comes from a fork -and to avoid any leaks-, Github doesn't seem to be forwarding the secret values. I thought that when a maintainer (me) triggers it manually the secrets were passed but that doesn't seem to be the case. I'll investigate to check what I could do in PRs from forks (most likely skip the related tests).

But anyway, PR is good and related test worked so let's merge it! :)

@Wauplin Wauplin merged commit 4f1e4bb into huggingface:main Jul 10, 2023
@dulayjm
Copy link
Contributor Author

dulayjm commented Jul 10, 2023

Awesome, thank you so much!

@julien-c
Copy link
Member

This is really cool! Random unsollicited question, did you try out the huggingface.js implementation? https://huggingface.co/docs/huggingface.js/inference/classes/HfInference#objectdetection

(same client, but in JS)

@dulayjm
Copy link
Contributor Author

dulayjm commented Jul 10, 2023

Thank you!!

No, I haven't checked out the JS library, but I'll give it a look! 🤗

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