Accept any non-string iterable for distutils.Extension's sources
#311
Conversation
|
I have a bunch of failing tests, linter errors, and several warnings from VS Code extensions locally; I hope the CI will be cleaner in its output. :) |
agriyakhetarpal
force-pushed
the
types/extension-sources
branch
from
e7aba2d to
115bb67
Compare
distutils/extension.py
Outdated
| raise AssertionError("'name' must be a string") # noqa: TRY004 | ||
| if not ( | ||
| isinstance(sources, list) | ||
| and all(isinstance(v, (str, os.PathLike)) for v in sources) | ||
| ): | ||
|
|
||
| # we handle the string case first; though strings are iterable, we disallow them | ||
| if isinstance(sources, str): | ||
| raise AssertionError( # noqa: TRY004 | ||
| "'sources' must be an iterable of strings or PathLike objects, not a string" | ||
| ) | ||
|
|
||
| # mow we check if it's iterable and contains valid types | ||
| try: | ||
| sources = list(sources) # convert to list for consistency | ||
| if not all(isinstance(v, (str, os.PathLike)) for v in sources): | ||
| raise AssertionError( | ||
| "All elements in 'sources' must be strings or PathLike objects" | ||
| ) | ||
| except TypeError: | ||
| raise AssertionError( |
There was a problem hiding this comment.
Question for the distutils maintainers: I can't help but feel that AssertionError is misused here, shouldn't these all be TypeError ? (it's noqa'd but not explained)
There was a problem hiding this comment.
I have the same suggestion! :)
There was a problem hiding this comment.
Peeking at the history, these used to be actual assert statements (7d70ca7), so my guess is that the AssertionErrors were kept for backwards compatibility with code that might try to catch these errors.
There was a problem hiding this comment.
Additionally, what I infer from the commit message is that that they were added because in-line assert statements are completely removed from the bytecode if used with -O
Co-Authored-By: Avasam <[email protected]>
Co-authored-by: Avasam <[email protected]>
|
To expand on the additional context: This allows typing the |
Description
This PR continues what was discussed in python/typeshed#12958 (comment) on allowing a loosened check for the source files for a
distutilsExtensioninstance.Previously: just lists were accepted; now: any non-string iterable is accepted (tuples, sets, other iterators, etc.). I plan to update python/typeshed in accordance with this change.
Additional context
xref: python/mypy#18107, python/typeshed#12958