fixing the duplicate header error logging - #1093
Devansh-567 wants to merge 1 commit into
Conversation
Signed-off-by: Devansh-567 <devansh.jay.singh@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. Summary by CodeRabbit
WalkthroughThe ChangesSpreadsheet error formatting
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~2 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The duplicate-header error is now logged without masking the original spreadsheet error. The focused and full test suites pass. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
PR fix
While reviewing the spreadsheet error handling, I noticed that the duplicate-header error handler in read_spreadsheet() was passing a value to logger.error() without a %s placeholder in the message.
This caused the logging call itself to raise a TypeError instead of showing the duplicate header, which masked the original spreadsheet error.
issue is here:
logger.error(
"If this exception says you have a duplicate cell name, the duplicate is",
findDups(wsh.row_values(1)),
)
The message needs a
%splaceholder for the duplicate value.Testing
I verified that test_read_spreadsheet_duplicate_headers now passes.
The full test suite passes with: