Add full type annotations to MIDI#1922
Merged
Merged
Conversation
Annotate every parameter and return type across the MIDI implementation (base, percussion, realtime.StreamPlayer, translate). All four pass mypy under --disallow-untyped-defs --disallow-incomplete-defs. Notable supporting changes (all behavior-preserving, verified by the existing doctests and tests.py): - base.MidiFile.open() now accepts str|pathlib.Path (it is already called with Path objects). - The three meta-event readers narrow with isinstance(eventList, MidiEvent) instead of common.isListLike(), giving mypy proper narrowing. - assignPacketsToChannels() uses track=None for the conductor pitch-bend event (the inline comment already stated this intent; the track is overwritten by MidiTrack.updateEvents() before serialization) and skips tracks with no assigned channel. - midiAsciiStringToBinaryString() guards against tracksEventsList=None. - percussion attribute accesses that live on Instrument subclasses are narrowed with TYPE_CHECKING asserts. Test code (tests.py and the disabled x_test* scaffolds in realtime) is left untyped, by request. AI-assisted (Claude)
Convert the `if t.TYPE_CHECKING: assert isinstance(...)` narrowings added in the previous commit to the preferred `t.cast(...)`-to-new-variable form (percussion.py: 2 spots; translate.py: 4 spots). This narrows for both mypy and ty (the assert form did not persist across re-reads of a @Property under ty), and is the style the project is standardizing on. mypy and ruff clean; tests.py and doctests pass. AI-assisted (Claude)
After merging master, the deprecated midiEventsToInstrument() (decorated
with @common.deprecated('v10', 'v11', ...)) has been removed, so `common`
was no longer referenced in real code -- only in doctests, which resolve it
via the music21 package namespace, not this module's import. ruff F401 and
pylint W0611 (CI lint/ruff jobs) flagged it on the PR's merge commit.
AI-assisted (Claude)
|
coverage: 93.103% (-0.003%) from 93.106% — worktree-type_midi into master |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add much more typing to the MIDI module. Use cast() for type narrowing instead of assert isinstance()