Add __len__ wrapping for C++ container classes#223
Conversation
Implement the wrap-len annotation which allows C++ classes with container
interfaces to expose Python's __len__ special method. This enables len()
calls on wrapped objects.
Changes:
- Add wrap_len attribute to ResolvedClass in DeclResolver.py
- Add __len__ code generation in CodeGenerator.py
- Add comprehensive tests covering:
- Basic wrap-len with size() method
- Different method names (length(), count(), getSize())
- Container without wrap-len (no __len__ generated)
- wrap-len with wrap-ignored size() method (still works)
- Template classes with wrap-len
- Choosing between multiple methods (size vs length)
- Boolean context (empty containers are falsy)
- Edge cases (empty, large containers)
Usage in pxd files:
cdef cppclass Container:
# wrap-len:
# size()
size_t size()
📝 WalkthroughWalkthroughThis PR implements a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/test_wrap_len.py (1)
116-121: Consider renaming unused loop variable.The loop variable
iis unused since the same item is appended repeatedly. While functionally correct, renaming to_would clarify intent and silence the static analysis warning.# Add multiple items - for i in range(100): + for _ in range(100): container.append(b"item")tests/test_files/wrap_len_test.hpp (1)
23-23: Consider bounds checking for robustness (optional).The
get()method accessesdata_[index]without bounds checking, which could cause undefined behavior if called with an out-of-bounds index. While acceptable for test code where inputs are controlled, adding.at(index)would provide safer behavior with automatic bounds checking.- int get(size_t index) const { return data_[index]; } + int get(size_t index) const { return data_.at(index); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.gitignore(1 hunks)autowrap/CodeGenerator.py(1 hunks)autowrap/DeclResolver.py(2 hunks)tests/test_files/wrap_len_test.hpp(1 hunks)tests/test_files/wrap_len_test.pxd(1 hunks)tests/test_wrap_len.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/test_wrap_len.py (2)
autowrap/__init__.py (1)
parse_and_generate_code(106-119)tests/test_files/wrap_len_test.hpp (14)
BasicContainer(13-13)BasicContainer(14-18)LengthContainer(31-31)i(84-84)i(84-84)CountContainer(43-43)CountContainer(44-44)IgnoredSizeContainer(74-74)IgnoredSizeContainer(75-79)GetSizeContainer(92-92)GetSizeContainer(93-93)EmptyContainer(102-102)DualLenContainer(124-124)DualLenContainer(125-129)
autowrap/CodeGenerator.py (1)
autowrap/Code.py (1)
add(56-75)
🪛 Ruff (0.14.8)
tests/test_wrap_len.py
118-118: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: test (==3.1.0, 3.11)
- GitHub Check: test (==3.2.0, 3.13)
- GitHub Check: test (==3.1.0, 3.10)
- GitHub Check: test (==3.2.0, 3.10)
- GitHub Check: test (==3.2.0, 3.11)
- GitHub Check: test (==3.1.0, 3.12)
- GitHub Check: test (==3.2.0, 3.12)
- GitHub Check: test (==3.1.0, 3.13)
🔇 Additional comments (9)
.gitignore (1)
176-176: LGTM!The generated wrapper file is correctly added to the gitignore, consistent with other test wrapper files in this section.
autowrap/DeclResolver.py (1)
179-179: LGTM!The
wrap_lenattribute is correctly added toResolvedClass, following the same pattern as the existingwrap_hashattribute. The type annotation and initialization are consistent with the codebase conventions.Also applies to: 210-210
tests/test_wrap_len.py (3)
31-45: LGTM!The module fixture correctly uses autowrap's
parse_and_generate_codeandcompile_and_importutilities to build the test wrapper. The fixture scope is appropriately set to "module" for efficiency.
146-154: Verify unsigned underflow behavior.The test assumes
decrement()clamps at 0 for the unsigned return type. This relies on the C++ implementation checkingif (count_ > 0)before decrementing (as seen in wrap_len_test.hpp line 48). The test correctly validates this safety behavior.
276-305: LGTM - Good test for method selection.This test validates that
wrap-lencorrectly uses the specified method (length()which returnssize * 2) rather than defaulting tosize(). This is important for verifying the annotation system works as intended.tests/test_files/wrap_len_test.hpp (1)
1-138: LGTM - Well-designed test fixtures.The C++ container classes provide good coverage of the wrap-len scenarios:
- Various method naming conventions (
size(),length(),count(),getSize())- Template containers
- Method selection between multiple candidates (
DualLenContainer)- Edge cases (always-empty, wrap-ignored methods)
The classes are simple and focused on testing the annotation feature.
autowrap/CodeGenerator.py (1)
687-697: No changes needed. The generated__len__method is syntactically correct. Thewrap_len[0]value contains the full method call syntax including parentheses (e.g.,size()), as specified in the wrap-len annotation format. The code at lines 687-697 generates correct Cython syntax:return deref(self.inst.get()).size().Likely an incorrect or invalid review comment.
tests/test_files/wrap_len_test.pxd (2)
1-6: LGTM: File header and imports are well-structured.The Cython language level directive and standard library imports are appropriate for this test file.
8-110: Excellent test coverage for wrap-len feature.The test file provides comprehensive coverage of wrap-len scenarios:
- Basic usage with
size()method- Alternative method names (
length(),count(),getSize())- Absence of wrap-len annotation
- Interaction with
wrap-ignore- Template classes
- Multiple candidate methods
The class declarations are well-structured and the comments clearly explain the intent of each test case. The corresponding C++ header file contains proper implementations for all test classes.
|
I thought more about wrap-vector-member. Then Len is Automatically member.size() etc. |
|
And when you do wrap-vector-interface, it will call size on the class itself. And all the other functions, too. |
|
Annotation on class level. Didn't think it 100% through though |
|
Ok I leave this for the next autowrap update |
3 participants
Implement the wrap-len annotation which allows C++ classes with container interfaces to expose Python's len special method. This enables len() calls on wrapped objects.
Changes:
Usage in pxd files:
cdef cppclass Container: # wrap-len:
# size()
size_t size()
Summary by CodeRabbit
New Features
__len__()method generation for Python compatibility. Supports multiple length-reporting method names (size(), length(), count(), getSize()).Tests
✏️ Tip: You can customize this high-level summary in your review settings.