Skip to content

Conversation

@ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Nov 9, 2023

What does this PR do?

#25598 uses from pytest import mark which breaks the function get_test_classes as the test module would have mark as attribute, and

getattr(attr_value, "all_model_classes", [])

gives an 'MarkDecorator' object (while attr_value is the pytest.mark).

This PR import pytest and use @pytest.mark to avoid the issue.

@ydshieh ydshieh changed the title Fix get_test_classes Fix the function get_test_classes Nov 9, 2023
@ydshieh ydshieh requested a review from amyeroberts November 9, 2023 10:03
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Nov 9, 2023

The documentation is not available anymore as the PR was closed or merged.

@amyeroberts
Copy link
Contributor

Would just using pytest.mark.flash_attn_test instead of importing from pytest import mark work? This looks like a complex workaround for a feature which is just a nice-to-have e.g. (comments here and here)

@ydshieh
Copy link
Collaborator Author

ydshieh commented Nov 9, 2023

Thanks for the great suggestion ! It works, and I will apply the same changes to all related test files.

@ydshieh ydshieh force-pushed the fix_tiny_model_creation branch from 6aa89cd to 235a8be Compare November 9, 2023 11:11
Copy link
Contributor

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

@ydshieh ydshieh changed the title Fix the function get_test_classes use pytest.mark directly Nov 9, 2023
@ydshieh ydshieh merged commit 3258ff9 into main Nov 9, 2023
@ydshieh ydshieh deleted the fix_tiny_model_creation branch November 9, 2023 12:32
EduardoPach pushed a commit to EduardoPach/transformers that referenced this pull request Nov 19, 2023
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