marco-c / autowebcompat

Automatically detect web compatibility issues
Mozilla Public License 2.0
34 stars 41 forks source link

Fix newline issue on Windows #249

Closed amaaniqbal closed 6 years ago

amaaniqbal commented 6 years ago

This pull request fixes extra generated newline on Windows while writing to csv file. The generated newline was causing error while reading from csv file.

codecov-io commented 6 years ago

Codecov Report

Merging #249 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #249   +/-   ##
=======================================
  Coverage   23.05%   23.05%           
=======================================
  Files          11       11           
  Lines        1301     1301           
  Branches      179      179           
=======================================
  Hits          300      300           
  Misses        999      999           
  Partials        2        2
Impacted Files Coverage Δ
autowebcompat/utils.py 50.64% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1f43667...9d4b172. Read the comment docs.

marco-c commented 6 years ago

Are you sure this is a problem? For reading, we are opening the file with newline=None, so it means universal newline mode is enabled, which means all newlines are translated to \n.

amaaniqbal commented 6 years ago

I guess so because for reading if there is an extra newline then error is being thrown and that extra newline is generated while writing to csv file.

Atleast this has been the case with me and doing above step solved the issue on Windows. You can browse through https://stackoverflow.com/questions/3348460/csv-file-written-with-python-has-blank-lines-between-each-row/3348664 for details.

Its better to see its behavior on other OS too.

marco-c commented 6 years ago

OK, makes sense. Please switch the csv reading to use newline='' too, as that's suggested by the Python documentation: https://docs.python.org/3/library/csv.html#id3.

propr[bot] commented 6 years ago

Please provide your feedback on this pull request here.

Privacy statement: We don't store any personal information such as your email address or name. We ask for GitHub authentication as an anonymous identifier to account for duplicate feedback entries and to see people specific preferences.