January typing #1201
January typing #1201
Conversation
jacobtylerwalls
force-pushed
the
january-typing
branch
from
28b3f54
to
a05bf2b
Compare
There was a problem hiding this comment.
Thanks for the great work. It'll need changes though to be accepted.
I can't overstress how much I dislike bare asserts and how I don't want them in music21. (I hate them more than mypy hates hasattr). In nearly every case where they appear, I would prefer to have type checking disabled on the parent function (return "Any") than have users need to deal with asserting that things not supported in this version of Python (like Literal[False]) don't apply.
Maybe we can put this into a m21 v8 branch, which won't support Python 3.7, and that will have fewer cases of teaching the system about something explained in the docs, but here I count one possible bug that mypy found, compared to at least 9 workarounds for bugs/deficiencies in their own system.
Mypy conformance might have been something easy to get in if music21 were being started from scratch in 2022, but at this point there are so much code history that cannot cleanly be brought into line with what mypy wants that I'm not sure it's going to be possible to get everything into the typechecking system until they become more flexible on what they're able to read.
| @@ -1168,7 +1187,7 @@ def getContextByClass( | |||
| sortByCreationTime=False, | |||
| followDerivation=True, | |||
| priorityTargetOnly=False, | |||
| ) -> Optional['Music21Object']: | |||
| ) -> Optional[_M21T]: | |||
| @@ -810,7 +813,7 @@ def _augmentOrDiminishTuple(self, amountToScale): | |||
| return durationTupleFromQuarterLength(self.quarterLength * amountToScale) | |||
|
|
|||
|
|
|||
| DurationTuple.augmentOrDiminish = _augmentOrDiminishTuple | |||
| DurationTuple.augmentOrDiminish = _augmentOrDiminishTuple # type: ignore[attr-defined] | |||
There was a problem hiding this comment.
Maybe we need to subclass a Mixin or something? I'm never sure how namedtuples work.
There was a problem hiding this comment.
Thanks for the patient work on this! :-)
I think that it's time to release a v7.3 with the changes so far and go straight to a making master a v8 branch for any future typings that allows for the Literal[...] type since we'll make 3.8 the lowest supported system. (a nice thing: we've now got SemVer aligned with minimum Python version).
| @@ -191,7 +191,7 @@ def __init__(self, | |||
| figure: Optional[str] = None, | |||
| root: Optional[pitch.Pitch] = None, | |||
| bass: Optional[pitch.Pitch] = None, | |||
| inversion: Optional[pitch.Pitch] = None, | |||
| inversion: Optional[int] = None, | |||
All for now, just wanted to see what other "little" stuff there was that didn't involve the huge modules.