KEMBAR78
Fix check_all_models_are_tested by ydshieh · Pull Request #16970 · huggingface/transformers · GitHub
Skip to content

Fix check_all_models_are_tested #16970

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 27, 2022
Merged

Fix check_all_models_are_tested #16970

merged 1 commit into from
Apr 27, 2022

Conversation

@ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Apr 27, 2022 •
edited
Loading

What does this PR do?

The block from L396 to L398 should be in the else block if I understand correctly.

transformers/utils/check_repo.py

Lines 394 to 398 in 8d3f952

else:
test_file = test_file[0]
new_failures = check_models_are_tested(module, test_file)
if new_failures is not None:
failures += new_failures

Otherwise, when a model has no test file, I get errors like below, and the program stops immediately.
(with test_file = [] passed to check_models_are_tested)

  File "/home/yih_dar_huggingface_co/transformers/utils/check_repo.py", line 362, in check_models_are_tested
    tested_models = find_tested_models(test_file)
  File "/home/yih_dar_huggingface_co/transformers/utils/check_repo.py", line 343, in find_tested_models
    with open(os.path.join(PATH_TO_TESTS, test_file), "r", encoding="utf-8", newline="\n") as f:
  File "/usr/lib/python3.9/posixpath.py", line 90, in join
    genericpath._check_arg_types('join', a, *p)
  File "/usr/lib/python3.9/genericpath.py", line 152, in _check_arg_types
    raise TypeError(f'{funcname}() argument must be str, bytes, or '
TypeError: join() argument must be str, bytes, or os.PathLike object, not 'list'

@ydshieh ydshieh changed the title fix Fix check_all_models_are_tested Apr 27, 2022
@ydshieh ydshieh requested a review from sgugger April 27, 2022 18:09
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Apr 27, 2022 •
edited
Loading

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

sgugger
sgugger approved these changes Apr 27, 2022
View reviewed changes
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Looks like an indentation mistake indeed. Thanks for fixing!

@ydshieh ydshieh merged commit 596afb4 into huggingface:main Apr 27, 2022
@ydshieh ydshieh deleted the fix_check_all_models_are_tested branch April 27, 2022 19:18
chamidullinr pushed a commit to chamidullinr/transformers that referenced this pull request Apr 28, 2022
@ydshieh @chamidullinr
Fix check_all_models_are_tested (huggingface#16970)
0dcd254 
Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
elusenji pushed a commit to elusenji/transformers that referenced this pull request Jun 12, 2022
@ydshieh @elusenji
Fix check_all_models_are_tested (huggingface#16970)
12f745f 
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

Reviewers

1 more reviewer

@sgugger sgugger sgugger approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ydshieh @HuggingFaceDocBuilderDev @sgugger