Skip to content

Conversation

@ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Feb 19, 2025

What does this PR do?

Fix #36267.

Marian have a few tests regarding its conversion file, but in the release branch, we remove those files and tests are failing.
I don't have clear context if we should keep these (3) tests, and marian seems to be the only model where have tests involving conversion files.

In order to be run CI with v4.49-release, we will need a patch however.

cc @Rocketknight1 for visibility.

@ydshieh ydshieh changed the title fix Fix broken CI due to missing conversion files Feb 19, 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
Contributor

@dvrogozh dvrogozh left a comment

Choose a reason for hiding this comment

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

I guess alternate solution is to separate this test out to dedicated test file and prune it along with conversion scripts on the release commit. However, I am not aware of the plans around conversion scripts in general, so I just note a possibility. Are they being deprecated and will be pruned going forward? Regardless, this change works for me in the sense that on future Transformers releases we will be able to run tests w/o explicitly ignoring some of them.

@ydshieh
Copy link
Collaborator Author

ydshieh commented Feb 19, 2025

We are removing conversion scripts in the release as those cause some security reports and prevent users from enterprises from using transformers. Let's see what the other maintainers say 🤗

@Rocketknight1
Copy link
Member

I think we can just remove the tests for conversion files entirely!

@ydshieh
Copy link
Collaborator Author

ydshieh commented Feb 20, 2025

Merge it. It's only about conversion files and marian is the only model that has such tests.

@ydshieh ydshieh merged commit e8531a0 into main Feb 20, 2025
17 checks passed
@ydshieh ydshieh deleted the fix_ci_with_conv branch February 20, 2025 12:22
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.

ci/v4.49-release: tests collection fail with "No module named 'transformers.models.marian.convert_marian_to_pytorch'" on v4.48/v4.49 release branches

5 participants