Conversation
Signed-off-by: Nikola Forró <nforro@redhat.com> Assisted-by: Claude Opus 4.6 via Claude Code
|
Build succeeded. ✔️ pre-commit SUCCESS in 2m 16s |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Sections class to support lookups by both normalized ID and normalized name in the __contains__ and find methods. A new unit test has been added to verify this behavior. The review feedback suggests refining the __contains__ implementation to safely handle non-string inputs, such as Section objects, to avoid potential AttributeError exceptions when calling .lower().
| id_lower = cast(str, id).lower() | ||
| return any( | ||
| s.normalized_id == id_lower or s.normalized_name == id_lower for s in data | ||
| ) |
There was a problem hiding this comment.
The __contains__ method currently assumes that the id argument is always a string and calls .lower() on it. Since Sections inherits from UserList[Section], it is expected that __contains__ can also handle Section objects (e.g., when checking section_obj in sections). If a non-string object is passed, this will raise an AttributeError because of the .lower() call.
It is recommended to check if id is a string first, and fall back to a standard list containment check otherwise.
| id_lower = cast(str, id).lower() | |
| return any( | |
| s.normalized_id == id_lower or s.normalized_name == id_lower for s in data | |
| ) | |
| if not isinstance(id, str): | |
| return id in data | |
| id_lower = id.lower() | |
| return any( | |
| s.normalized_id == id_lower or s.normalized_name == id_lower for s in data | |
| ) |
3 participants
Fixes #417.