We use the default configuration of DictWriter, which is tuned to be close to what Excel expects. Excel being a product from the Windows world, \r\n is used instead of just \n.

Now changing that would probably break compatibility with quite a few integrations. I think we should keep the default as it looks sane (it's the default of most applications generating CSV as far as I can tell), and trimming the CRLF can be done with Unix tools.

Thanks for the comment @k4nar. I believe Excel is compatible with \n (only have LibreOffice installed to test and it generates \n by default), but if there are already integrations relying on the \r\n options, then maybe I could add a config option for this instead?

I made a suggestion for a configurable \n line terminator in my latest commits.

I have mixed feelings about this... I think most UNIX users are able to use dos2unix or sed magic to fix their CSV (if required). Maybe I'm a little biased about this.

Yes it is true that it is possible to use those tools. I personally think it would be more convenient to have it as an option in the configuration. Do you think there is a drawback with adding it, like not cluttering the config file with too many possible options?

Do you think there is a drawback with adding it, like not cluttering the config file with too many possible options?

Yup.

Makes sense, what do you think about setting the lineterminator to os.linesep instead of adding a config option? That way, users of either platform would get what they expect. This is how the Python data analysis library pandas handles writing of csv files:

line_terminator str, optional
The newline character or character sequence to use in the output file. Defaults to os.linesep, which depends on the OS in which this method is called (‘n’ for linux, ‘rn’ for Windows, i.e.).

Good idea! I wonder why this is not the default for the csv module.

Ok for me 👍 .

Great! I added this change and removed the previous ones.

Thanks @davidag, I updated the two tests that used a lineterminator variable explicitly and added a note in the changelog.

Hey @joelostblom can you rebase your branch so that I can merge this. Thanks!

@jmaupetit Rebased!

🤔 I guess a new arrow release is breaking the build... again. 😞

Yes, i submitted another PR to pin the arrow version #372

Yup saw it, merged it! Ready for another rebase?

Done!