Refactor/harmony - #547
Merged
Merged
Conversation
azfoo
force-pushed
the
refactor/harmony
branch
2 times, most recently
from
June 2, 2026 10:18
3a3b7a0 to
de6557a
Compare
3 tasks
azfoo
force-pushed
the
refactor/harmony
branch
2 times, most recently
from
July 21, 2026 11:34
3a4e416 to
b4ee754
Compare
the if block prevented applied_to from clearing once it was set through the text box.
The test was silently never receiving the qtui fixture (its first parameter was bound to self instead), so it only passed by riding on Get.MAIN_WINDOW state left over from earlier tests in the same session. It failed outright when run in isolation.
Deriving inversion counts purely from music21's CHORD_TYPES tone counts let the Inspector and Add Harmony dialog offer non-root inversions for Italian/French/German sixths, the Neapolitan, the Tristan chord, and power chords, even though to_roman_numeral/letter_symbol_label render a fixed string for these regardless of bass note, making the control a silent no-op. Document per-quality why each is locked to root position and, for Neapolitan and power (the two that do have real inversions in principle), what's actually blocking support: Neapolitan's "N6" abbreviation in music21 isn't the classical triad at all but an unrelated 4-note all-interval tetrachord, and power chords can't be inverted through music21's normal API and have no established figured-bass notation.
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
1 participant
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.
letter_symbolnow catchesChordExceptionand falls back to root position when music21 cannot construct the requested inversion (e.g.minor-sixthat inversion 3). Closes 3rd Inversion of Minor 6th chord crashes the app #376.NOTE_NAME_TO_INT,INT_TO_NOTE_NAME, etc.) are now derived from music21 at import time rather than duplicated by hand;_INV_TO_STRINGis the single source of truth for inversion labels across the dialog and inspector.