Fix strict none errors in the test subpackage. #2200
Conversation
This shows several different approaches to dealing with #2199 for p[i].arg.
| sources.append(BuildSource(program_path, module_name, program_text)) | ||
| sources.append(BuildSource(program_path, module_name, | ||
| None if incremental else program_text)) | ||
| res = None |
There was a problem hiding this comment.
This refactor was needed because program_text is previously inferred as str, and that's correct. So this is technically a case of reusing the same variable name with a different type (#1174). [UPDATE: This comment should be one line up.]
| @@ -53,11 +53,11 @@ def parse_test_cases( | |||
| while i < len(p) and p[i].id != 'case': | |||
| if p[i].id == 'file': | |||
| # Record an extra file needed for the test case. | |||
| files.append((os.path.join(base_path, p[i].arg), | |||
| files.append((os.path.join(base_path, p[i].arg or ''), | |||
There was a problem hiding this comment.
Should this really be the empty string, or is this an error condition?
There was a problem hiding this comment.
IIUC the '' is unreachable -- when id is 'file' arg is never None. A different rewrite would be
arg = p[i].arg
assert arg is not None
There was a problem hiding this comment.
Hm, I rewrote it using the assert, then realized that arg may be None if someone puts [file] in a testcase. That's nonsense. In general the parsing of test cases is pretty crummy, so I don't consider it my job to fix it.
There was a problem hiding this comment.
Agreed. Having an assertion failure in that case seems fine to me.
|
Not sure we can add that mypy.ini, because suppressing Optional errors suppresses other kinds of errors too... |
|
Adding those flags to mypy.ini should be fine, since they don't have any effect unless you also pass |
|
LGTM! |
This shows several different approaches to dealing with #2199 for
p[i].arg.@ddfisher Thoughts? Maybe we can also commit a mypy.ini containing