Skip to content

Fix __contains__ and __eq__ override signatures#649

Open
bizfsc wants to merge 1 commit intocanopen-python:masterfrom
bizfsc:fix/mypy-override-signatures
Open

Fix __contains__ and __eq__ override signatures#649
bizfsc wants to merge 1 commit intocanopen-python:masterfrom
bizfsc:fix/mypy-override-signatures

Conversation

@bizfsc
Copy link
Copy Markdown
Contributor

@bizfsc bizfsc commented Apr 29, 2026

Fix __contains__ and __eq__ override signatures to satisfy the Liskov substitution principle.

Problem

mypy reports [override] errors because __contains__ and __eq__ methods use specific types (Union[int, str], ODRecord, etc.) instead of object as required by the Mapping, Container, and object supertypes.

Changes

canopen/objectdictionary/__init__.py

  • ObjectDictionary.__contains__: accept object instead of Union[int, str]
  • ODRecord.__contains__: accept object instead of Union[int, str]
  • ODRecord.__eq__: accept object, return NotImplemented for non-ODRecord
  • ODArray.__eq__: accept object, return NotImplemented for non-ODArray
  • ODVariable.__eq__: accept object, return NotImplemented for non-ODVariable

canopen/sdo/base.py

  • SdoBase.__contains__: accept object instead of Union[int, str]
  • SdoRecord.__contains__: accept object instead of Union[int, str]
  • SdoArray.__contains__: accept object instead of int, add isinstance guard

Impact

Eliminates all 18 [override] mypy errors (95 → 77 remaining).

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

❌ Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
canopen/sdo/base.py 80.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Change method signatures to accept 'object' as required by the
Liskov substitution principle (Mapping/Container/object supertypes).

For __eq__, return NotImplemented for incompatible types instead of
raising AttributeError. For __contains__ in SdoArray, add isinstance
check to handle non-int arguments gracefully.
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.

1 participant