Claude/add stl container tests #220

timosachsenberg · 2025-11-29T13:05:37Z

Two-member hash function test
Tests that hash uses both value_ AND name_ members:

item_alice = m.Item(100, b"alice")
item_bob = m.Item(100, b"bob") # Same value_, different name_
items = {item_alice, item_bob}
assert len(items) == 2 # Both are distinct!

Bug fix: Variable name collision
Fixed ConversionProvider.py where for key, value in m.items(): could overwrite function parameters named key.

Summary by CodeRabbit

Tests
- Enhanced wrapped container test coverage to validate Python dict and set semantics.
- Added validation for hash and equality behavior with wrapped container items.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…collision bug - Add tests for hash-based lookups in unordered_map (find, count, at) - Add tests for membership checks in unordered_set (count, find) - Add lookup tests for wrapped class containers (map, unordered_map, unordered_set) - Fix bug where loop variables 'key'/'value' could shadow function parameters of the same name in StdMapConverter and StdUnorderedMapConverter The new tests verify that dict/set operations actually use hashing for O(1) lookups rather than just iterating through all items.

- Update Item hash function to combine both value_ and name_ members - Add test verifying that items with same value_ but different name_ are treated as distinct (tests proper multi-field hashing)

- Rename test files: new_stl_test.* -> cpp17_stl_test.* - Rename test module: test_code_generator_new_stl.py -> test_code_generator_cpp17_stl.py - Rename class: NewSTLTest -> Cpp17STLTest - Add user-centric documentation with container mappings and usage examples - Document Python types for each C++ container (dict, set, list, T|None, bytes) - Include example syntax for passing/receiving containers - Document compilation requirements (C++17)

- Add wrap-hash annotation and operator==/!= to Item class for Python hashability - Add getHashValue() method that combines both value_ and name_ for proper hashing - Update tests to use Python d[key] and 'in' operator instead of iteration - Remove unnecessary C++ lookup functions (lookupMapIntToItem, hasKeyMapIntToItem, etc.) - Tests now verify that wrapped classes work correctly as Python dict keys This enables Python code like: result = t.createMapItemToInt(3) item = m.Item(1) assert result[item] == 10 # Direct Python dict lookup!

coderabbitai · 2025-11-29T13:05:47Z

Warning

Rate limit exceeded

@timosachsenberg has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 40 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 042bf81 and c72e46c.

📒 Files selected for processing (1)

tests/test_wrapped_containers.py (5 hunks)

Walkthrough

The changes add hash and equality comparison support to the wrapped Item class and extend test coverage to validate Python's native dictionary and set semantics for wrapped containers, including hash-based containment checks and equality-based item distinction.

Changes

Cohort / File(s)	Summary
C++ Header methods `tests/test_files/wrapped_container_test.hpp`	Added `operator!=` for comparison negation and `getHashValue()` accessor to compute combined hash from `value_` and `name_` fields.
Cython declarations `tests/test_files/wrapped_container_test.pxd`	Exposed C++ methods as Cython declarations: `operator==`, `operator!=`, and `getHashValue()` for the Item class.
Python tests `tests/test_wrapped_containers.py`	Refactored tests to use Python's native dict-key access and membership operators (`in`). Added validation of hash/equality semantics for distinct items with same `value_` but different `name_` fields.
Generated code `tests/test_files/cpp17_stl_test.pyx`	Updated autowrap version comment from 0.24.0 to 0.23.0 (no functional changes).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

tests/test_wrapped_containers.py: Verify new test patterns correctly exercise hash/equality semantics for map and unordered_set lookups, especially logic distinguishing items with identical value_ but different name_ fields.
tests/test_files/wrapped_container_test.hpp: Confirm getHashValue() hash computation combines both fields consistently and operator!= is logically correct.
tests/test_files/wrapped_container_test.pxd: Validate Cython declarations match C++ signatures and enable proper wrapping.

Possibly related PRs

Add lookup/membership tests for STL containers and fix variable name collision bug #216: Adds/modifies STL lookup and membership tests, introduces hash and equality support for wrapped containers, touching the same test files and Item class APIs.

Suggested reviewers

jpfeuffer

Poem

🐰 A rabbit hops through hashes bright,
Items equal, sets all right,
Dicts now speak the Python way,
Wrapped containers hop and play!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 68.75% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Claude/add stl container tests' is partially related to the changeset - it mentions adding STL container tests which is reflected in the test file changes, but it doesn't capture the main additions of hash/equality operator implementations and the bug fix in ConversionProvider.py.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

…unGiJjFeExriA

timosachsenberg · 2025-11-29T13:08:00Z

tests/test_wrapped_containers.py

+        # Direct Python dict lookup with wrapped class key - NOT iteration!
+        key0 = m.Item(0)
+        key1 = m.Item(1)
+        key2 = m.Item(2)
+        assert result[key0] == 0
+        assert result[key1] == 10
+        assert result[key2] == 20
+
+        # Test 'in' operator
+        assert key1 in result
+        missing = m.Item(999)
+        assert missing not in result


@jpfeuffer like this?

darauf nen Caipi. :) Danke!

Test verifies that Python dict d[key] lookup works correctly when keys are wrapped classes with multiple members (value_ and name_). Both members must match for the lookup to succeed.

claude added 5 commits November 28, 2025 23:33

Add two-member hash function test for unordered_set

3094352

- Update Item hash function to combine both value_ and name_ members - Add test verifying that items with same value_ but different name_ are treated as distinct (tests proper multi-field hashing)

Add generated cpp17_stl_test.pyx

a72596f

Merge branch 'master' into claude/add-stl-container-tests-01SghEieKPb…

042bf81

…unGiJjFeExriA

timosachsenberg commented Nov 29, 2025

View reviewed changes

Add d[key] lookup test with multi-member wrapped class keys

c72e46c

Test verifies that Python dict d[key] lookup works correctly when keys are wrapped classes with multiple members (value_ and name_). Both members must match for the lookup to succeed.

timosachsenberg requested a review from jpfeuffer November 29, 2025 13:23

timosachsenberg merged commit 0501161 into master Nov 29, 2025
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Claude/add stl container tests #220

Claude/add stl container tests #220

Uh oh!

timosachsenberg commented Nov 29, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Nov 29, 2025 •

edited

Loading

Rate limit exceeded

Uh oh!

timosachsenberg Nov 29, 2025

Uh oh!

jpfeuffer Nov 29, 2025

Uh oh!

timosachsenberg Nov 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Claude/add stl container tests #220

Claude/add stl container tests #220

Uh oh!

Conversation

timosachsenberg commented Nov 29, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Nov 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

timosachsenberg Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

jpfeuffer Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

timosachsenberg Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

timosachsenberg commented Nov 29, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Nov 29, 2025 •

edited

Loading