Skip to content

Conversation

@remi-or
Copy link
Collaborator

@remi-or remi-or commented May 28, 2025

This PR adds the require_speech decorator and adds it to three tests in seamless_m4t and seamless_m4t_v2 that fail with an ImportError if is_speech_available() == False

@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.

@mht-sharma mht-sharma requested a review from ydshieh May 28, 2025 11:23
@Rocketknight1
Copy link
Member

cc @ydshieh

"""
Decorator marking a test that requires speech. These tests are skipped when speech isn't available.
"""
return unittest.skipUnless(is_speech_available(), "test requires speech")(test_case)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here we have to decide what we really require.

def is_speech_available():
    # For now this depends on torchaudio but the exact dependency might evolve in the future.
    return _torchaudio_available

and in setup.py we have

extras["speech"] = deps_list("torchaudio") + extras["audio"]

If we want to require torchaudio + audio for the tests, then is_speech_available is not enough.
Also the message test requires speech will be confusing as speech is not a python library.

If we only want to require torchaudio, then we have def require_torchaudio(test_case):.

cc @eustlb

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it would be nice to figure out if is_speech_available is needed or redundant. If it isn't, I will change the decorator to require_torchaudio. I have no idea on this though.
As for the message, we can change it to when the packages required for speech models are not available ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am happy to merge for now if the message is changed to

test requires torchaudio

And come back to check the details later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

@ydshieh ydshieh enabled auto-merge (squash) June 2, 2025 09:05
@ydshieh ydshieh merged commit 493cf15 into huggingface:main Jun 2, 2025
20 checks passed
@remi-or remi-or mentioned this pull request Jun 4, 2025
mht-sharma pushed a commit that referenced this pull request Jun 4, 2025
* [janus] Fix failing tests on mi3XX (#38426)

* Fix multiple devices error on Janus

* Fix AttributeError on Janus BOI token

* Initialize lm first in Janus to get correct device map

* Added expectations for Janus test_model_generate_images

* Fixed JanusVisionEncoderLayer being split across devices

* Code formatting

* Adding modeling file

* Reverted changes out of scope for this PR

* [seamless_m4t] Skip some tests when speech is not available (#38430)

* Added the require_speech decorator

* Added require_speecj to some seamless_m4t tests

* Changed skip message
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