make shape points into tuples #61
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes in this pull request focus on the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (1)
datumaro/components/annotation.py (1)
Line range hint
642-646: Fix typo in position setter method name.The method name
_set_poistioncontains a typo and should be_set_position.Apply this fix:
- def _set_poistion(self, value): + def _set_position(self, value):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
datumaro/components/annotation.py(1 hunks)
🔇 Additional comments (1)
datumaro/components/annotation.py (1)
537-538: LGTM! Converting points to tuple improves data integrity.
The change from List[float] to Tuple[float] makes the points immutable, which is a good practice for geometric data that shouldn't change after creation.
Let's verify that all shape subclasses handle immutable points correctly:
✅ Verification successful
Based on the search results, I can now provide a conclusive response about the points attribute change:
Converting points to tuple is safe and well-supported across the codebase
The codebase analysis shows that all shape classes (Bbox, Polygon, PolyLine, and Points) handle points consistently:
- They only read point values without modifying them
- Point modifications happen during object creation, not after
- Operations like merging and matching create new objects instead of modifying existing ones
Key evidence:
- No direct point mutations found in any shape class
- All point manipulations use list comprehensions or numpy operations that create new arrays
- Merger and matcher classes properly handle immutable points by creating new annotations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all classes that inherit from _Shape and verify their point manipulation
ast-grep --pattern 'class $name($_Shape) {
$$$
def $method($args) {
$$$
$_.points$_
$$$
}
$$$
}'
Length of output: 118
Script:
#!/bin/bash
# Let's try a different approach to find shape-related classes and their point usage
# First, find all Python files
echo "=== Files containing '_Shape' class or inheritance ==="
rg -l "_Shape" --type py
echo -e "\n=== Code snippets showing point manipulation ==="
rg "\.points" --type py -B 2 -A 2
echo -e "\n=== Class definitions inheriting from _Shape ==="
rg "class.*\([^)]*_Shape[^)]*\)" --type py -B 1 -A 3
Length of output: 54287
|
Summary
How to test
Checklist
developbranchLicense
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation