bpo-30157: Fix csv.Sniffer.sniff() regex pattern#1273
Conversation
Looks like there is an extra > in the regex pattern for this sort of csv line: `# ,".*?"`
|
@jake-jake-jake, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rhettinger, @benjaminp and @birkenfeld to be potential reviewers. |
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
|
Please open a bug report to describe the bug and link it to this PR: then add "bpo-xxx" the PR title. |
> in sniffer regex pattern for sniffer> in sniffer regex pattern for sniffer bpo-30157
> in sniffer regex pattern for sniffer bpo-30157> in sniffer regex pattern for sniffer
> in sniffer regex pattern for sniffer > in sniffer regex pattern for sniff
|
@Haypo Let me know if anything else needs adjusting. |
|
Change title to something like |
|
I'm surprized that such bug was not caught by tests. This means the lack of tests. Needed a new test for that case. |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Needed a test and a Misc/NEWS entry.
> in sniffer regex pattern for sniff|
I added some tests for the regex patterns that should ensure that Sniffer._guess_quote_and_delimiter() matches patterns as expected. |
|
@serhiy-storchaka Let me know if I need to make any more amendments to the tests or news entry; the tests should properly identify when the patterns fail to match. |
|
|
|
To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request. If/when the requested changes have been made, please leave a comment that says, |
|
Recreated as #5601. |
Looks like there is an extra > in the regex pattern for this sort of csv line:
# ,".*?"