Skip to content

Conversation

@gante
Copy link
Contributor

@gante gante commented Feb 17, 2025

What does this PR do?

generate has a few things that are shared across frameworks. Sometimes, when making core changes, we need to update all three frameworks. Otherwise, we get a red CI (example).

Since we're deprecating TF/JAX, this PR removes their tests regarding generate so that the other frameworks no longer hinder PT-related generate development.

cc @LysandreJik @ArthurZucker as we chatted on Slack 🤗


@pytest.mark.generate
@require_torch
class GenerationIntegrationTests(unittest.TestCase, GenerationIntegrationTestsMixin):
Copy link
Contributor Author

@gante gante Feb 17, 2025

Choose a reason for hiding this comment

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

GenerationIntegrationTestsMixin contained integration tests shared across frameworks. They were moved here. A few redundant tests were removed in the process (e.g. max_new_tokens checks, which are part of the tests in GenerationTesterMixin)

Copy link
Collaborator

Choose a reason for hiding this comment

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

They were moved here

You mean those tests are moved to this class in this PR, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved from GenerationIntegrationTestsMixin [deleted] to GenerationIntegrationTests , yes :)

@gante gante marked this pull request as ready for review February 17, 2025 13:51
@gante gante requested a review from ydshieh February 17, 2025 13:51
@gante gante changed the title [TF/FLAX] remove tf/flax tests in /generation [tests] remove tf/flax tests in /generation Feb 17, 2025
@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.

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.

OK, especially you have talked to the core maintainers!

Thanks


@pytest.mark.generate
@require_torch
class GenerationIntegrationTests(unittest.TestCase, GenerationIntegrationTestsMixin):
Copy link
Collaborator

Choose a reason for hiding this comment

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

They were moved here

You mean those tests are moved to this class in this PR, right?

@gante gante merged commit 55493f1 into huggingface:main Feb 17, 2025
25 checks passed
@gante gante deleted the rm_flax_tf_gen_tests branch February 17, 2025 14:59
zucchini-nlp pushed a commit to zucchini-nlp/transformers that referenced this pull request Feb 21, 2025
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