feat: raise correct error when correlation writer file exists #107
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe pull request introduces a new, more specific exception class Changes
Possibly related PRs
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
🧹 Nitpick comments (1)
src/readii/io/writers/correlation_writer.py (1)
Line range hint
67-77: Update docstring to include new exception.The method's docstring should be updated to include
CorrelationWriterFileExistsErrorin its "Raises" section.Raises ------ CorrelationWriterIOError If an error occurs during file writing. + CorrelationWriterFileExistsError + If the output file already exists and overwrite is False. CorrelationWriterValidationError If the filename format is invalid.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
src/readii/io/writers/correlation_writer.py(2 hunks)
🔇 Additional comments (3)
src/readii/io/writers/correlation_writer.py (3)
23-27: Well-structured exception class!
The new CorrelationWriterFileExistsError class follows good practices:
- Proper inheritance hierarchy from
CorrelationWriterIOError - Clear and descriptive docstring
- Appropriate use as a marker exception
90-90: LGTM! Clear and helpful error handling.
The error handling change appropriately uses the new exception class with a helpful error message that guides users on how to resolve the issue.
Line range hint 23-90: Verify exception handling across the codebase.
Let's ensure that callers of this class properly handle the new exception type.
✅ Verification successful
No issues found with exception handling for CorrelationWriter
Based on the search results, the CorrelationWriter class and its exception types are properly handled:
- The test suite in
tests/io/writers/test_correlation_writer.pydemonstrates comprehensive exception handling:- Tests verify
CorrelationWriterValidationErrorfor invalid inputs - Tests verify
CorrelationWriterIOErrorfor file existence scenarios - Tests cover both overwrite=True and overwrite=False cases
- Tests verify
The changelog also indicates this was a planned feature with proper error handling implementation, and there are no direct usages of the class in the codebase yet besides tests.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of CorrelationWriter and exception handling patterns
echo "Searching for CorrelationWriter usage and exception handling..."
rg -l "CorrelationWriter" | while read -r file; do
if [[ "$file" != "src/readii/io/writers/correlation_writer.py" ]]; then
echo "=== $file ==="
# Show context around CorrelationWriter usage
rg -B 2 -A 5 "CorrelationWriter" "$file"
# Show any try-except blocks in the same file
rg -B 2 -A 5 "try:|except" "$file"
fi
done
Length of output: 5331
1 participant
added to make catching existing file error easier
Summary by CodeRabbit