Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove an irrelevant test from test_modeling_tf_layoutlm #14341

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Nov 9, 2021

What does this PR do?

This PR remove test_model_various_embeddings from test_modeling_tf_layoutlm.py.

More Information

Current version of test_modeling_tf_layoutlm.py has

def test_model_various_embeddings(self):
    config_and_inputs = self.model_tester.prepare_config_and_inputs()
    for type in ["absolute", "relative_key", "relative_key_query"]:
        config_and_inputs[0].position_embedding_type = type
        self.model_tester.create_and_check_model(*config_and_inputs)

After a quick search inside the repo, I believe position_embedding_type is only for (some) PyTorch models, and test_model_various_embeddings is not necessary here. Furthermore,

  • test_modeling_tf_layoutlm.py is the only TF test script containing position_embedding_type
  • modeling_tf_layoutlm.py has no usage of config.position_embedding_type.

Therefore, this PR remove test_model_various_embeddings from test_modeling_tf_layoutlm.py.

@ydshieh ydshieh changed the title remove test_model_various_embeddings from test_modeling_tf_layoutlm remove an irrelevant test from test_modeling_tf_layoutlm Nov 9, 2021
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Indeed, good catch @ydshieh!

@LysandreJik LysandreJik merged commit babd0b9 into huggingface:master Nov 9, 2021
@ydshieh ydshieh deleted the clean_tf_layoutlm_test branch November 9, 2021 16:43
Albertobegue pushed a commit to Albertobegue/transformers that referenced this pull request Jan 27, 2022
Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
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.

2 participants