wireservice / csvkit

A suite of utilities for converting to and working with CSV, the king of tabular file formats.
https://csvkit.readthedocs.io
MIT License
6.02k stars 603 forks source link

Carriage returns replaced with unix line-endings mid-field #606

Closed bbobbadi closed 7 years ago

bbobbadi commented 8 years ago

Hi I am using csvclean and csvcut tools, while using these tools the "Carriage Return" is removing from the field content, the CR should carry to the output file. Is there any way to retain these special character in the outputfile?

jpmckinney commented 8 years ago

Do you have a sample CSV I can test with?

bbobbadi commented 8 years ago

input: cut_test.csv csvcut -c COL2,COL3,COL4 cut_test.csv > cut1.csv output: cut1.csv csvcut_test.zip

If you see the output, the field content has been changed with respect to CR.

jpmckinney commented 8 years ago

I can reproduce. Using csv.reader reads \r\n. csvkit turns it into \n. I think this is an issue in agate. @onyxfish @nbedi

jpmckinney commented 8 years ago

Using latest agate, \r\n becomes \n.

onyxfish commented 7 years ago

Yeah this is weird. Ticketing upstream.

onyxfish commented 7 years ago

So I checked up the upstream code. Here is the relevant section:

    def writerow(self, row):
        if self.line_numbers:
            row = list(row)
            self._append_line_number(row)

        # Convert embedded Mac line endings to unix style line endings so they get quoted
        row = [i.replace('\r', '\n') if isinstance(i, six.string_types) else i for i in row]

        self.writer.writerow(row)

The short answer is this was done intentionally. I can't recall exactly the logic that led to it, but I do recall it was a compromise for a thorny issue that I don't particularly want to revisit. Turning \r\n into \n doesn't seem super problematic to me (it's still a line break), though I'm sure there are some weird edge-cases. Unless somebody can persuade me this is really important, I'm going to close it.

(Side note: That original test file has super-weird quoting, and also doesn't open correctly in Excel.)

jpmckinney commented 7 years ago

Sounds fair to me.

onyxfish commented 7 years ago

Closing this since there has been no disagreement.

CharlesNepote commented 7 years ago

@onyxfish and @jpmckinney

@onyxfish said:

Turning \r\n into \n doesn't seem super problematic to me (it's still a line break), though I'm sure there are some weird edge-cases. Unless somebody can persuade me this is really important, I'm going to close it.

RFC 4180 says CSV files should use \r\n (crlf) for the end of line. And some tools, at least CSVLint, strictly follow the RFC. For example: https://csvlint.io/validation/589e284f613562000400000f

I understand that the RFC is not a standard but why convert (crlf) to (lf) if the user made the choice of (crlf) at first? At least I could understand that csvclean doesn't convert (lf) to (crlf) nor (any-type-of-end-of-line) to (any-type-of-end-of-line).

I'm currently writing a document for thousands of futur Open Data producers in France (a new law made Open Data mandatory for public bodies with 3,500+ people). [Document is in french but I can publish a draft here.] I would like to promote csvkit but it's hard to do so until files produced by csvkit get a warning in CSVLint. These people doesn't have big skills with data and command line tools (and english): it has to be very simple.

At least it should be clearly documented. But I think it is better to be solved.

jpmckinney commented 7 years ago

@CharlesNepote This issue is about a situation like:

1,2,"3\r\n
4",5\n
6,7,8,9\n

Where the \r\n occurs within a field. It's not about \r\n as the line terminator. Please keep the discussion in #792.