refactor(typing): replace # type: ignores with # pyright: ignore[ruleName]; enable PGH003
#1381
Conversation
shiftinv
added
t: refactor/typing/lint
| self._retrieve_messages = self._retrieve_messages_around_strategy | ||
| if self.before and self.after: | ||
| self._filter = lambda m: self.after.id < int(m["id"]) < self.before.id # type: ignore | ||
| self._filter = lambda m: self.after.id < int(m["id"]) < self.before.id # pyright: ignore[reportOptionalMemberAccess] |
There was a problem hiding this comment.
These comments could be fixed by binding the narrowed self.before as a 2nd parameter to the lambda:
| self._filter = lambda m: self.after.id < int(m["id"]) < self.before.id # pyright: ignore[reportOptionalMemberAccess] | |
| self._filter = lambda m, b=self.before: self.after.id < int(m["id"]) < b.id |
...but that changes parameter count, so idk
There was a problem hiding this comment.
Submit a separate pull to fix these issues if you want to fix them.
disnake/poll.py
Outdated
| "duration": int(self.duration.total_seconds()) // 3600 | ||
| if self.duration is not None | ||
| else 24, |
There was a problem hiding this comment.
This technically changes the behavior (once discord adds non-expiring polls) but shrug
24 is the default duration
There was a problem hiding this comment.
This should be proactive and not change the behaviour, as if the behaviour is changed it becomes a different outcome
There was a problem hiding this comment.
I've changed it into an assert. Present code fails if it's None, so this should be a non-issue
|
Could you please resolve conflicts and enable ruff rule |
Enegg
changed the title
# type: ignores with # pyright: ignore[ruleName]# type: ignores with # pyright: ignore[ruleName]
Enegg
force-pushed
the
fix-type-ignores
branch
from
d3072df to
0f92a26
Compare
Enegg
changed the title
# type: ignores with # pyright: ignore[ruleName]# type: ignores with # pyright: ignore[ruleName]; enable PGH003
| "ANN0", | ||
| "ANN4", | ||
| ## flake8-bandit | ||
| "S101", # use of assert is fine |
There was a problem hiding this comment.
I'm not entirely sold on this change: if run with opitmisations then the asserts will be gone
There was a problem hiding this comment.
For shortcomings of Python's typing system I'd say asserts are fine, but for things that can actually happen (for instance, the API not returning values we'd expect, or incorrect usage of a method contrary to the docstring), an actual exception should be used, even if it's just raise AssertionError(...) so it can't be optimized away.
There was a problem hiding this comment.
Exactly this, asserts should only be used as an in-place alternative to typing.cast (where the lines being optimised out at runtime is actually beneficial), whereas any actual runtime exceptions should be raised. This matches Eneg's current implementation afaict at quick glance.
There was a problem hiding this comment.
I believe I've used asserts solely in places where the condition being false would result in the subsequent line failing.
For instance,
assert "foo" in typed_dict
typed_dict["foo"]There are places where T | None is passed to something accepting just T, but since it doesn't fail (immediately, anyway) at runtime I've preserved the # type: ignore
4 participants
Summary
Replaces bare
# type: ignorecomments with# pyright: ignore[specificRuleName, ...].Disables support for
# type: ignoreby pyright; this results in pyright suggesting the full diagnostic name(s) for autofixes.Some of the type ignores (mostly where type is
None-able, but None isn't expected) have been replaced withasserts.In a few places logic was fixed to avoid the comment entirely.
Checklist
pdm run nox -s lintpdm run nox -s pyright