transformers: Raise exceptions instead of using assertions for control flow
Assertions can’t be relied upon for control flow because they can be disabled, as per the following:
$ python --help
usage: python [option] ... [-c cmd | -m mod | file | -] [arg] ...
...
-O : remove assert and __debug__-dependent statements; add .opt-1 before
.pyc extension; also PYTHONOPTIMIZE=x
...
From my understanding, this is why mypy has no qualms about using them to narrow types because you can turn them off at runtime and so they incur zero cost.
Would you be open to me changing these assertions to other appropriate exceptions as I encounter them?
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 4
- Comments: 23 (14 by maintainers)
Commits related to this issue
- Assertions to exceptions (#13692) * Raise exceptions instead of using assertions for control flow #12789 * # coding=utf-8 * Raise exceptions instead of using assertions for control flow * Ra... — committed to huggingface/transformers by MocktaiLEngineer 3 years ago
- Assertions to exceptions (#13692) * Raise exceptions instead of using assertions for control flow #12789 * # coding=utf-8 * Raise exceptions instead of using assertions for control flow * Ra... — committed to Narsil/transformers by MocktaiLEngineer 3 years ago
- #12789 Replace assert statements with exceptions — committed to djroxx2000/transformers by djroxx2000 3 years ago
- #12789 Replace assert statements with exceptions (#13909) * #12789 Replace assert statements with exceptions * fix-copies: made copy changes to utils_qa.py in examples/pytorch/question-answering a... — committed to huggingface/transformers by djroxx2000 3 years ago
- Assertions to exceptions (#13692) * Raise exceptions instead of using assertions for control flow #12789 * # coding=utf-8 * Raise exceptions instead of using assertions for control flow * Ra... — committed to stas00/transformers by MocktaiLEngineer 3 years ago
- Replace assertions with ValueError exception (#14142) Updated masked-language modeling examples in pytorch with convention defined by #12789 — committed to huggingface/transformers by huberemanuel 3 years ago
- Raise exceptions instead of using asserts for control flow in modeling_openai #12789 — committed to nbertagnolli/transformers by nbertagnolli 3 years ago
- Raise exceptions instead of using asserts in modeling_openai #12789 (#14386) * Raise exceptions instead of using asserts for control flow in modeling_openai #12789 * reformatted file — committed to huggingface/transformers by nbertagnolli 3 years ago
- Assertions to exceptions (#13692) * Raise exceptions instead of using assertions for control flow #12789 * # coding=utf-8 * Raise exceptions instead of using assertions for control flow * Ra... — committed to Albertobegue/transformers by MocktaiLEngineer 3 years ago
- #12789 Replace assert statements with exceptions (#13909) * #12789 Replace assert statements with exceptions * fix-copies: made copy changes to utils_qa.py in examples/pytorch/question-answering a... — committed to Albertobegue/transformers by djroxx2000 3 years ago
- Replace assertions with ValueError exception (#14142) Updated masked-language modeling examples in pytorch with convention defined by #12789 — committed to Albertobegue/transformers by huberemanuel 3 years ago
- Raise exceptions instead of using asserts in modeling_openai #12789 (#14386) * Raise exceptions instead of using asserts for control flow in modeling_openai #12789 * reformatted file — committed to Albertobegue/transformers by nbertagnolli 3 years ago
The second approach is more informative indeed!
I’ve created PR #13894 . Please take a look.
@sgugger I am also going to pick several files to kill my time.
Just adding my two cents but I wouldn’t raise bare
Exceptions but instead raise the appropriate type of exception given the error.