Skip to content

Conversation

@gante
Copy link
Contributor

@gante gante commented Feb 20, 2025

What does this PR do?

  • adds smolvlm to doc toctree
  • adds num2words requirement

@gante gante requested a review from zucchini-nlp February 20, 2025 16:22
Copy link
Member

@zucchini-nlp zucchini-nlp left a comment

Choose a reason for hiding this comment

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

❤️

@Rocketknight1
Copy link
Member

We're also getting lots of CI failures because all of the processor tests for SmolVLM depend on num2words!

@gante gante changed the title [smolvlm] add smolvlm to docs toctree [smolvlm] make CI green Feb 20, 2025
@zucchini-nlp
Copy link
Member

Sorry, didn't notice it would fail. Actually the package is used only when chatting with video, so we might raise error only when num2words is actually called. And guard video chat template tests with num2-word_available. WDYT?

@gante
Copy link
Contributor Author

gante commented Feb 20, 2025

@zucchini-nlp at the end of the day, would fail too: either we have no tests for the feature, or we need to push the new requirement :D

I'm leaving that part of the logic changes to you 🤗 And I agree, the check should only exist where it is strictly needed

@HuggingFaceDocBuilderDev

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.

@zucchini-nlp
Copy link
Member

Yep, we can later raise error only when video code is called. But making num2words a requirements means users won't have a problem with the import error at all now :)

@gante gante requested a review from ydshieh February 20, 2025 16:58
@gante
Copy link
Contributor Author

gante commented Feb 20, 2025

Note: CI images being built here

Copy link
Collaborator

@ydshieh ydshieh left a comment

Choose a reason for hiding this comment

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

Sorry, I just realized that the file have to be changed is

docker/torch-light.dockerfile

(see slack message for more details)

@ydshieh ydshieh self-requested a review February 20, 2025 17:16
Copy link
Contributor

@molbap molbap left a comment

Choose a reason for hiding this comment

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

Processors seems green now!

@gante
Copy link
Contributor Author

gante commented Feb 20, 2025

CI images pushed in the push-ci-branch branch ✅

@ydshieh ydshieh merged commit 27d1707 into huggingface:main Feb 20, 2025
23 checks passed
@gante gante deleted the smolvlm_docs branch February 20, 2025 18:02
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.

6 participants