Type sequence checks in setuptools/dist.py#4578
Conversation
1cabeb2 to
d7db4a0
Compare
| 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.
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.
daf4485 to
7564ba0
Compare
7564ba0 to
000a413
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)