Skip to content

Fix OO operator protocols (NotImplemented, type checks, hash) - #10

Draft
endolith wants to merge 1 commit into
masterfrom
cursor/oo-operator-protocol-2589
Draft

endolith wants to merge 1 commit into
masterfrom
cursor/oo-operator-protocol-2589

Conversation

@endolith

Copy link
Copy Markdown
Owner

Summary

This addresses object-oriented / dunder-method issues in just_intonation.py: correct use of NotImplemented, type checks before touching the RHS, removal of dead state that caused misleading equality, and __hash__ where __eq__ is defined.

Issues fixed

  1. Interval.__add__ / __sub__
    When the right-hand side was not an Interval, these methods fell off the end and returned None, which breaks the numeric protocol and can surface as confusing TypeErrors. They now return NotImplemented.

  2. Interval rich comparisons and __eq__
    __lt__ / __gt__ / __le__ / __ge__ called _F(b) for arbitrary b, which could AttributeError (e.g. Interval(2, 1) < 2). They now require isinstance(b, Interval) and otherwise return NotImplemented.
    __eq__ now returns NotImplemented for non-Interval operands so reflexive equality is handled correctly instead of blindly returning False.

  3. Interval._terms
    It was only set on one constructor path, was never read anywhere in the repo, and happened to make Chord(M3) == Interval(5, 4) true in tests by accidental tuple equality. That assignment was removed so Interval does not carry misleading private state.

  4. Pitch.__init__
    After normalizing frequency, integral values were stored with int(frequency) (the original argument) instead of the coerced value. It now uses int(self._frequency), which is correct when frequency is already a Pitch or other wrapped type.

  5. Pitch ordering and __eq__
    Same pattern as Interval: isinstance(b, Pitch) and NotImplemented instead of b.frequency on arbitrary objects.

  6. Pitch.__hash__
    With __eq__ overridden, defining __hash__ keeps hashing behavior explicit and consistent with equality (based on stored frequency).

  7. Chord
    class Chord(): → class Chord:. __eq__ now checks isinstance(b, Chord) and returns NotImplemented otherwise (avoids AttributeError on b._terms). __hash__ added so chords can be used in sets/dicts like intervals.

Test change

  • Chord(M3) == Interval(5, 4) was only true because of the removed Interval._terms quirk. The test now asserts Chord(M3) != Interval(5, 4), which matches type-safe equality.

Verification

  • pytest (full suite): 21 passed
  • pytest --doctest-modules just_intonation.py: 17 passed
  • flake8 (syntax/undefined-name select on touched files): clean
Open in Web Open in Cursor 

- Interval: return NotImplemented from __add__/__sub__ when RHS is not an
  Interval; guard rich comparisons the same way; return NotImplemented from
  __eq__ for non-Intervals; drop unused Interval._terms assignment
- Pitch: normalize integral frequencies with int(self._frequency); guard
  ordering and __eq__ with isinstance; add __hash__ for consistency with __eq__
- Chord: use modern class syntax; guard __eq__ with isinstance; add __hash__
  based on terms
- Tests: Chord(M3) must not equal Interval(M3); that was accidental tuple
  equality via Interval._terms, not a supported API

Co-authored-by: endolith <endolith@gmail.com>
@cursor
cursor Bot force-pushed the cursor/oo-operator-protocol-2589 branch from 6f26c95 to 9278817 Compare June 12, 2026 04:44

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants