Fix doctest files fetch issue#23277
Conversation
|
The documentation is not available anymore as the PR was closed or merged. |
|
As said offline I don't think we need to revert urgently, we can just ignore the red check on main. |
There was a problem hiding this comment.
This is the stuff that probably needs to be different on main vs a PR branch. The test fetcher has this test that can be reused/adapted.
There was a problem hiding this comment.
The problems comes from the fact grep gives exit code 1 if the input is empty.
circleci@9576b4afebd1:~/transformers$ echo "" | grep -E '\.(mdx)$'
circleci@9576b4afebd1:~/transformers$ echo $?
1Considering the chain of 3 grep in the 2nd line here, I think it's best to have a python function to do things.
67a269a to
7ccebe6
Compare
ydshieh
changed the title
Refactor doctests + add CI to main"
There was a problem hiding this comment.
put the logic to some new functions in utils/tests_fetcher.py.
There was a problem hiding this comment.
just copies of existing functions with some modifications for doctest test files.
e8ee1c1 to
863dcf5
Compare
|
@sgugger FYI The doctest PR also has some problem running on our GitHub Actions runner. See error below. _____ ERROR collecting src/transformers/generation/configuration_utils.py ______
import file mismatch:
imported module 'transformers.generation.configuration_utils' has this __file__ attribute:
/transformers/src/transformers/generation/configuration_utils.py
which is not the same as the test file we want to collect:
/__w/transformers/transformers/src/transformers/generation/configuration_utils.py
HINT: remove __pycache__ / .pyc files and/or use a unique basename for your test file modules |
sgugger
left a comment
sgugger
left a comment
There was a problem hiding this comment.
Thanks for the cleanup! I just have a question on why the dummy.py. Maybe the comment can be more explicit on why it's needed?
@sgugger A short fix for this issue is given in the last commit. The reason is the file This change should be applied to other workflow file, but it's rare that we have such imports in the codebase. I will do it in a separate PR. |
|
Thanks for all the explanation! |
|
Going to merge as the failing tests are irrelevant and I have tried to re-run for a few times. |
* fix --------- Co-authored-by: ydshieh <[email protected]>
* fix --------- Co-authored-by: ydshieh <[email protected]>
3 participants
Reverts #23271
Embarrassingly and unfortunately, the new job
tests_pr_documientation_testsfails at the stepGet files to testwhen the job is run on themainbranch.https://app.circleci.com/pipelines/github/huggingface/transformers/64235/workflows/54a99003-258e-4c2a-8366-b4461b3ec33f/jobs/794628/parallel-runs/0/steps/0-113
I will have to take a look - the log is not informative.