{"action":"created","issue":{"url":"https://api.github.com/repos/wireservice/csvkit/issues/986","repository_url":"https://api.github.com/repos/wireservice/csvkit","labels_url":"https://api.github.com/repos/wireservice/csvkit/issues/986/labels{/name}","comments_url":"https://api.github.com/repos/wireservice/csvkit/issues/986/comments","events_url":"https://api.github.com/repos/wireservice/csvkit/issues/986/events","html_url":"https://github.com/wireservice/csvkit/issues/986","id":363849331,"node_id":"MDU6SXNzdWUzNjM4NDkzMzE=","number":986,"title":"Improve dialect sniffing","user":{"login":"akkartik","id":3161,"node_id":"MDQ6VXNlcjMxNjE=","avatar_url":"https://avatars3.githubusercontent.com/u/3161?v=4","gravatar_id":"","url":"https://api.github.com/users/akkartik","html_url":"https://github.com/akkartik","followers_url":"https://api.github.com/users/akkartik/followers","following_url":"https://api.github.com/users/akkartik/following{/other_user}","gists_url":"https://api.github.com/users/akkartik/gists{/gist_id}","starred_url":"https://api.github.com/users/akkartik/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akkartik/subscriptions","organizations_url":"https://api.github.com/users/akkartik/orgs","repos_url":"https://api.github.com/users/akkartik/repos","events_url":"https://api.github.com/users/akkartik/events{/privacy}","received_events_url":"https://api.github.com/users/akkartik/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-09-26T03:59:50Z","updated_at":"2018-09-26T14:52:01Z","closed_at":null,"author_association":"NONE","body":"Back in June 2017, @petersonjr pointed out some test cases that cause csvkit/agate to misinterpret the dialect of files: https://github.com/wireservice/csvkit/issues/751#issuecomment-310803282\r\n\r\nThe reasonable workaround @jpmckinney pointed out was to configure `--snifflimit 0` at the commandline. While this helps, I'd like to point out a couple of things:\r\n\r\na) Those test cases work fine without the override to the `Sniffer` class introduced in https://github.com/wireservice/agate/commit/3b9ceea131ba143cc72b6d1a9f7871d059188b52\r\n\r\nb) I dug into [the default Sniffer in the Python standard library](https://github.com/python/cpython/blob/3.7/Lib/csv.py#L218). It's not very robust; it uses regular expressions for parsing each line, which means that it gets confused by say commas inside quoted strings.\r\n\r\nc) While agate has a reasonable default `sniff_limit` of 0, the commandline argument parsing in csvkit overrides it to `None`, which has the effect of reading the entire file. As the size of the input file grows, it becomes increasingly likely that it'll encounter stuff that the Python Sniffer can't handle.\r\n\r\nThese observations indicate a few places in the stack where a change may be beneficial. I'm not sure if any of them is a good idea -- I may well be missing context as an outsider -- but thought I'd try to start a discussion. Is there a testsuite for the custom Sniffer in agate? Is it an option to default `--snifflimit` to something smaller, say 1024? (Obviously changing the Python standard library is a bigger task. [The maintainer for the csv module is also not active anymore.](https://devguide.python.org/experts))"},"comment":{"url":"https://api.github.com/repos/wireservice/csvkit/issues/comments/424745593","html_url":"https://github.com/wireservice/csvkit/issues/986#issuecomment-424745593","issue_url":"https://api.github.com/repos/wireservice/csvkit/issues/986","id":424745593,"node_id":"MDEyOklzc3VlQ29tbWVudDQyNDc0NTU5Mw==","user":{"login":"jpmckinney","id":26463,"node_id":"MDQ6VXNlcjI2NDYz","avatar_url":"https://avatars1.githubusercontent.com/u/26463?v=4","gravatar_id":"","url":"https://api.github.com/users/jpmckinney","html_url":"https://github.com/jpmckinney","followers_url":"https://api.github.com/users/jpmckinney/followers","following_url":"https://api.github.com/users/jpmckinney/following{/other_user}","gists_url":"https://api.github.com/users/jpmckinney/gists{/gist_id}","starred_url":"https://api.github.com/users/jpmckinney/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jpmckinney/subscriptions","organizations_url":"https://api.github.com/users/jpmckinney/orgs","repos_url":"https://api.github.com/users/jpmckinney/repos","events_url":"https://api.github.com/users/jpmckinney/events{/privacy}","received_events_url":"https://api.github.com/users/jpmckinney/received_events","type":"User","site_admin":false},"created_at":"2018-09-26T14:52:01Z","updated_at":"2018-09-26T14:52:01Z","author_association":"MEMBER","body":"There are some sniffing tests in agate:\r\n\r\nhttps://github.com/wireservice/agate/blob/master/tests/test_py2.py\r\nhttps://github.com/wireservice/agate/blob/master/tests/test_py3.py\r\n\r\ncsvkit sets the sniff limit to `None` to maintain the same behavior as earlier versions.\r\n\r\nIt would be reasonable to allow users to change this default, which is the subject of #974.\r\n\r\nWhat other changes were you considering?"}}
{"actor":{"display_login":"jpmckinney"}}