array spool indexing #491
array spool indexing #491
Conversation
WalkthroughThis update extends the data selection capabilities in the spool handling classes. The Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant B as BaseSpool
participant D as DataFrameSpool
U->>B: Call __getitem__(item)
alt item is np.ndarray
B->>B: Check is_array(item)
B->>D: Call _select_from_array(item)
D-->>B: Return new spool instance
else item is int/slice
B->>B: Process simple index/slice
end
B-->>U: Return selected spool
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
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
🧹 Nitpick comments (1)
docs/tutorial/spool.qmd (1)
104-133: Great addition of numpy-style array indexing examples!The examples effectively demonstrate how to use boolean and integer arrays with spools, aligning the spool behavior with numpy's interface. This will be helpful for users familiar with numpy.
However, there's a small inconsistency in the variable naming in the integer array example:
-# Get an array of integers which indicate the new order or patches -bool_array = np.array([2, 0]) +# Get an array of integers which indicate the new order or patches +int_array = np.array([2, 0]) -# create a new spool with patch 2 and patch 0 in that order. -new = spool[bool_array] +# create a new spool with patch 2 and patch 0 in that order. +new = spool[int_array]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dascore/core/spool.py(4 hunks)docs/tutorial/spool.qmd(1 hunks)tests/test_core/test_spool.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: test_code_min_deps (ubuntu-latest, 3.13)
- GitHub Check: test_code_min_deps (windows-latest, 3.12)
- GitHub Check: test_code_min_deps (macos-latest, 3.13)
- GitHub Check: test_code_min_deps (windows-latest, 3.13)
- GitHub Check: test_code_min_deps (ubuntu-latest, 3.12)
- GitHub Check: test_code_min_deps (macos-latest, 3.12)
- GitHub Check: test_code (windows-latest, 3.11)
- GitHub Check: test_code (ubuntu-latest, 3.12)
- GitHub Check: test_code (macos-latest, 3.11)
- GitHub Check: test_code (windows-latest, 3.10)
- GitHub Check: test_code (macos-latest, 3.10)
- GitHub Check: test_code (macos-latest, 3.12)
- GitHub Check: test_code (windows-latest, 3.12)
- GitHub Check: test_code (ubuntu-latest, 3.11)
- GitHub Check: test_code (ubuntu-latest, 3.10)
🔇 Additional comments (5)
dascore/core/spool.py (3)
60-60: Improved type hint for__getitem__method.The updated type hint properly documents that the method can handle slices and numpy arrays in addition to integers.
380-399: Well-implemented array selection logic.The implementation handles both boolean and integer arrays appropriately:
- Boolean arrays filter patches directly with the boolean mask
- Integer arrays select specific patches by index position
- The method correctly raises an error for unsupported array types
The method also preserves the source dataframe and instructions, maintaining consistency with the rest of the codebase.
412-413: Clean integration of array selection.The addition to
__getitem__effectively integrates the array selection functionality while maintaining compatibility with existing indexing behavior.tests/test_core/test_spool.py (2)
182-206: Comprehensive testing for boolean array selection.The test class thoroughly covers boolean array selection with tests for:
- All true values (maintaining the original spool)
- All false values (resulting in an empty spool)
- Mixed boolean values (correctly filtering specific patches)
The tests verify both the length of the resulting spool and the content equality with the expected dataframe, providing robust validation.
208-236: Well-designed tests for integer array selection.The test class efficiently covers various integer array scenarios:
- Standard sequential ordering
- Error handling for out-of-bounds indices
- Error handling for incorrect array types
- Reordering of patches based on custom indices
The tests cover both normal usage and edge cases, ensuring reliability of the implementation.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #491 +/- ##
=======================================
Coverage 99.85% 99.85%
=======================================
Files 118 118
Lines 9696 9711 +15
=======================================
+ Hits 9682 9697 +15
Misses 14 14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Description
This PR enables indexing a spool with a numpy array, which is the same behavior as numpy arrays. For example, boolean arrays can be used to "deselect" patches like so:
and integer arrays can be used to re-arrange patches:
Checklist
I have (if applicable):
Summary by CodeRabbit