Type sequence checks in setuptools/dist.py #4578
Conversation
Avasam
force-pushed
the
type-setuptools-dist-sequence-methods
branch
2 times, most recently
from
1cabeb2
to
d7db4a0
Compare
| @@ -799,7 +819,7 @@ def _include_misc(self, name, value): | |||
| ) | |||
| else: | |||
| new = [item for item in value if item not in old] | |||
| setattr(self, name, old + new) | |||
| setattr(self, name, list(old) + new) | |||
There was a problem hiding this comment.
By adding annotations to the method, mypy now raises this issue. This fixes a TypeError when a Distribution's old included attribute was a tuple.
abravalheri
force-pushed
the
type-setuptools-dist-sequence-methods
branch
from
d7db4a0
to
daf4485
Compare
| due_date=(2025, 8, 28), # Originally added on 2024-08-27 | ||
| ) | ||
| return _sequence | ||
| raise AttributeError(f"module {__name__!r} has no attribute {name!r}") |
There was a problem hiding this comment.
@Avasam, how about this for the deprecation of sequence?
After a quick search on github, I do see people accessing it, so we cannot simply remove it...
There was a problem hiding this comment.
I had opted out of wanting to replacesequence by typing.Sequence in #4575 (comment) because there's a lot of implications when it comes to new checks and supported types (like having to explicitely check for str first everytime, because a str is a valid Sequence[str], both at runtime and in type-checking).
Not to say it wouldn't be an improvement, just that's not something I wanna go into for the scope of those PRs.
Is making sequence private something you want to do independent of those changes ? If so, this commit is fine yeah.
There was a problem hiding this comment.
Is making sequence private something you want to do independent of those changes ?
I think so. It seems to be such a trivial implementation detail, I don't know why people are importing it from setuptools.
It is probably good to deprecate it now just in case at some point we decide to remove it.
Avasam
force-pushed
the
type-setuptools-dist-sequence-methods
branch
from
daf4485
to
7564ba0
Compare
Summary of changes
Split from #4575
Fixes a
TypeErrorwhen aDistribution's old included attribute was atuple.Note that I'm purposefully avoiding using
Iterable,SequenceorContainerbecausestrmatches all those. And I didn't wanna complicate the checks to allow "any iterable except str" (maybe in a future PR)Pull Request Checklist
newsfragments/.(See documentation for details)