Skip to content

feat: Use csv.DictReader to parse header fields (msto#19)#20

Open
bostonrwalker wants to merge 1 commit intomsto:mainfrom
bostonrwalker:feat_msto19
Open

feat: Use csv.DictReader to parse header fields (msto#19)#20
bostonrwalker wants to merge 1 commit intomsto:mainfrom
bostonrwalker:feat_msto19

Conversation

@bostonrwalker
Copy link

Used csv.DictReader() to do the dirty work because RFC4180 has lots of tricky rules around quoting and special characters. Added a test to demonstrate. Tested using Python 3.10, 3.11, and 3.12.

@msto
Copy link
Owner

msto commented Nov 29, 2024 via email

Copy link
Owner

@msto msto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks again for the PR.

I think I should have initially included the dialect parameter in the Reader and Writer classes, and permitted other arguments such as quoting to be passed through as kwargs, similar to DictReader's interface.

https://docs.python.org/3/library/csv.html#csv.DictReader

Would there be any reasons to favor keeping quoting as an explicit argument if that change were made?

@bostonrwalker
Copy link
Author

I think I only included it for consistency. **kwargs should do the job.

@msto
Copy link
Owner

msto commented Dec 9, 2024

Cool. I made the change in #21. Let me know if it addresses #19 to your satisfaction and I'll merge it in later this week after I clean up the docs.

Thanks again for the PR - this was a smart change 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants