Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork3.1k
Allow trailing commas forfiles setting inmypy.ini andsetup.ini#18621
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Diff frommypy_primer, showing the effect of this PR on open source code: imagehash (https://github.com/JohannesBuchner/imagehash): 1.56x faster (39.4s -> 25.2s in a single noisy sample) |
| """ | ||
| returnsplit_and_match_files_list(paths.split(",")) | ||
| returnsplit_and_match_files_list(split_commas(paths)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Maybe do the same for pyproject.toml? (Line 202 usestry_split and is equally prone to trailing comma)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I tried this test:
[case testCmdlineCfgFilesTrailingComma]# cmd: mypy[file pyproject.toml]\[tool.mypy]files = ["a.py","b.py",][file a.py]x:str = 'x'# ok[file b.py]y:int = 'y'# E: Incompatible types in assignment (expression has type "str", variable has type "int")[file c.py]# This should not trigger any errors, because it is not included:z:int = 'z'[out]
and it passed onmain. So, I don't think that this change is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nah, not exactly. This is perverse style, but...
[case testCmdlineCfgFilesTrailingComma]# cmd: mypy[file pyproject.toml]\[tool.mypy]files = """ a.py, b.py,"""[file a.py]x: str = 'x' # ok[file b.py]y: int = 'y' # E: Incompatible types in assignment (expression has type "str", variable has type "int")[file c.py]# This should not trigger any errors, because it is not included:z: int = 'z'[out](remove the trailing comma to make it pass)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
When lists are toml lists, commas are handled much earlier by the parser itself. Butmypy seems to support ini-style comma-delimited lists as single strings as well. I don't know how useful that is, but let's keep commas treatment consistent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ok, got it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ok, here's the thing.pyproject.toml usestry_split function which is regex based.
Looks like there are multiple settings that are affected.
Let's do it this way: since I cannot really change just one handler, I will merge this PR and create the next one with many tests based ontry_split.
5ee02cd intomasterUh oh!
There was an error while loading.Please reload this page.
…i` (python#18621)Now ```inifiles = a.py, b.py```and ```inifiles = a.py, b.py,```will be the same thing.Previously, adding a traling comma would add `''` to `paths`, whichresulted in a strange error like:``` a.py: error: Duplicate module named "a" (also at "a.py") (diff) a.py: note: Seehttps://mypy.readthedocs.io/en/stable/running_mypy.html#mapping-file-paths-to-modules for more info (diff) a.py: note: Common resolutions include: a) using `--exclude` to avoid checking one of them, b) adding `__init__.py` somewhere, c) using `--explicit-package-bases` or adjusting MYPYPATH (diff)```Refspython#14240Refspython/cpython#129708
…roject.toml` (python#18624)Refspython#18621Closespython#18623With a lot more tests.
Uh oh!
There was an error while loading.Please reload this page.
Now
files = a.py, b.pyand
files = a.py, b.py,will be the same thing.
Previously, adding a traling comma would add
''topaths, which resulted in a strange error like:Refs#14240
Refspython/cpython#129708