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 debug script to check inference snippets + update Python text-to-image snippets #994

Closed
wants to merge 12 commits into from

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Oct 29, 2024

Reviewing PRs that updates the inference snippets is a bit painful / error-prone given that we can't easily check the diff.
This PR tries to solve this a bit with a new command that prints the inference snippets to the terminal. I also (tried) to add an automated check in the CI to avoid figuring it out manually. Let's see if it proves useful.

I also started to update the Python inference snippets, starting with the text-to-image one to return both requests and huggingface_hub versions.

Note: the update of packages/tasks/pnpm-lock.yaml is far too much. I'll check if I can't minimize it (I only needed to add a dev dependency). => removed the dependency in b3deb60 (no real need for it)

Snippets can be checked in https://github.com/huggingface/huggingface.js/actions/runs/11579670253/job/32236421855?pr=994.

@Wauplin Wauplin marked this pull request as ready for review October 29, 2024 17:55
@Wauplin Wauplin requested a review from mishig25 October 29, 2024 17:55
});
},
{
client: "huggingface_hub",
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe let's agree on the convention of putting huggingface_hub as the first item in the list

@@ -0,0 +1,55 @@
/*
Copy link
Collaborator

@mishig25 mishig25 Oct 31, 2024

Choose a reason for hiding this comment

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

I've implemented tests in https://github.com/huggingface/huggingface.js/pull/1003/files using the vite test convention of xyz.spec.ts files (which run on pnpm test).

I think we should just put more tests into xyz.spec.ts files rather than creating a custom mechanism of check-snippet.ts & inference-check-snippets.yml. Or am I missing some necessary details?

Wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

much better yes!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with this solution because I was bored about how difficult it was to debug things locally but automated tests feel much more natural now that you mention it ^^

@Wauplin
Copy link
Contributor Author

Wauplin commented Nov 5, 2024

closing in favor of #1016

@Wauplin Wauplin closed this Nov 5, 2024
Wauplin added a commit that referenced this pull request Nov 14, 2024
Supersedes #994.

This PR adds an `huggingface_hub` snippet for `text-to-image` inference
in Python. I added a test as done in
#1003.

Once this one is approved and merged, I'll move on with all other tasks
that the `InferenceClient` supports.

---------

Co-authored-by: Mishig <[email protected]>
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.

3 participants