-
Notifications
You must be signed in to change notification settings - Fork 12
rcv: Improve CSV parsing preprocessing and logging #838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #838 +/- ##
===========================================
+ Coverage 87.43% 87.82% +0.38%
===========================================
Files 38 38
Lines 3176 3187 +11
Branches 321 323 +2
===========================================
+ Hits 2777 2799 +22
+ Misses 257 249 -8
+ Partials 142 139 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ed1661b to
a4afa22
Compare
- Implement preprocessing for required fields in CSV parsing - Improve error handling for CSV row data conversion Ref: https://app.shortcut.com/cordada/story/15534/
a4afa22 to
4d74644
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances CSV parsing by stripping out empty required fields before schema validation and improving error handling/logging for row conversion failures.
- Introduces a
preprocesshook to remove empty or null required fields prior to validation. - Refactors conversion exception logging to include row index and renames the error key to
conversion_errors. - Adds unit tests and a CSV fixture to verify missing required fields and conversion errors in the “venta” parser.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/tests/test_rcv_parse_csv.py | Added tests for missing required fields and conversion error scenarios in parse_rcv_venta_csv_file. |
| src/tests/test_data/sii-rcv/RCV-venta-missing-required-fields.csv | New CSV fixture with intentionally empty required fields for testing preprocessing logic. |
| src/cl_sii/rcv/parse_csv.py | Implemented preprocess to strip empty required fields and improved conversion error logging and error-key naming. |
Comments suppressed due to low confidence (2)
src/cl_sii/rcv/parse_csv.py:1201
- [nitpick] The key 'conversion_errors' is plural but holds a single error string; consider renaming it to 'conversion_error' or storing a list of errors to match the plural form.
row_errors['conversion_errors'] = conversion_error
src/tests/test_rcv_parse_csv.py:157
- This test is currently a stub and does not validate any behavior; consider implementing tests for missing required fields and conversion errors in the 'compra' CSV parsing to match the coverage added for 'venta'.
def test_parse_rcv_compra_registro_csv_file(self) -> None:



Ref: https://app.shortcut.com/cordada/story/15534/