Skip to content

Fix OD item lookup evaluating object's length (fixes #588) #589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 15, 2025

Conversation

sveinse
Copy link
Collaborator

@sveinse sveinse commented Jun 14, 2025

Here is a proposal for a fix

Fixes #588

Copy link

codecov bot commented Jun 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@acolomb acolomb changed the title Fix __getitem__ from OD without using subobject's __len__ Fix OD item lookup evaluating subobject's length (fixes #588) Jun 15, 2025
@acolomb acolomb changed the title Fix OD item lookup evaluating subobject's length (fixes #588) Fix OD item lookup evaluating object's length (fixes #588) Jun 15, 2025
@acolomb acolomb merged commit de0e644 into canopen-python:master Jun 15, 2025
4 of 5 checks passed
@sveinse
Copy link
Collaborator Author

sveinse commented Jun 15, 2025

Thanks! I am not permitted to make the merge even you approved it @acolomb, so I don't have all permissions required it seems.

@sveinse sveinse deleted the fix-incorrect-get branch June 15, 2025 18:54
@acolomb
Copy link
Member

acolomb commented Jun 15, 2025

Thanks, good oberservation and a clean fix.

Please do propose a description of the change as well next time. Referring to the fixed issue number alone doesn't cut it when looking at the Git log.

@sveinse
Copy link
Collaborator Author

sveinse commented Jun 15, 2025

Ok, understood. Another thing that should be documented in "how we do things" as this differs between projects :D

@acolomb
Copy link
Member

acolomb commented Jun 15, 2025

Thanks! I am not permitted to make the merge even you approved it @acolomb, so I don't have all permissions required it seems.

Sorry about that. I can't see why not, will have to investigate a little.

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.

Incorrect fetch from the OD objects due to their __len__ method
2 participants