Skip to content

Fix #2050: Fix __eq__ implementations#2067

Open
mdboom wants to merge 2 commits into
NVIDIA:mainfrom
mdboom:__eq__-overload
Open

Fix #2050: Fix __eq__ implementations#2067
mdboom wants to merge 2 commits into
NVIDIA:mainfrom
mdboom:__eq__-overload

Conversation

@mdboom
Copy link
Copy Markdown
Contributor

@mdboom mdboom commented May 12, 2026

This bug was found with the static type-checking in #2061.

__eq__ overrides have to handle /any/ type being passed into them, and should not blindly access members of what is passed in.

@github-actions github-actions Bot added the cuda.core Everything related to the cuda.core module label May 12, 2026
@github-actions
Copy link
Copy Markdown

@mdboom mdboom self-assigned this May 12, 2026
@mdboom mdboom added bug Something isn't working P0 High priority - Must do! labels May 12, 2026
@mdboom mdboom added this to the cuda.core next milestone May 12, 2026
@mdboom mdboom requested a review from Andy-Jost May 13, 2026 12:49
Copy link
Copy Markdown
Contributor

@rwgk rwgk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor GPT-5.4 Extra High Fast


Findings

  • No issues found. For the scope of PR 2067, the tests look reasonably complete.
  • cuda_core/tests/test_event.py covers both sides of the IPCEventDescriptor.__eq__ fix: unrelated-type comparisons now follow Python's rich-comparison protocol, and same-type comparisons still preserve the intended value semantics.
  • cuda_core/tests/test_strided_layout.py does the same for _StridedLayout: it exercises ==, !=, reverse comparison, and the direct __eq__ NotImplemented contract, plus a same-type equality/inequality smoke test.

Assumptions

  • cuda_core/tests/test_object_protocols.py does not include IPCEventDescriptor or _StridedLayout in the shared equality matrix, so these focused tests are the primary regression coverage for those two types.
  • That looks acceptable for this PR because the production change is narrowly scoped to those two __eq__ implementations.

Verification

  • Ran TestVenv/bin/python -m pytest cuda_core/tests/test_event.py -q and got 28 passed, 2 skipped.
  • Ran TestVenv/bin/python -m pytest cuda_core/tests/test_strided_layout.py -q and got 689 passed.
  • The added coverage matches the regression described in issue #2050 and also checks that same-type equality behavior remains intact.

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

Labels

bug Something isn't working cuda.core Everything related to the cuda.core module P0 High priority - Must do!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants