Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG] add proper newline output for csv module #1319

Merged
merged 5 commits into from
Mar 12, 2021
Merged

[MRG] add proper newline output for csv module #1319

merged 5 commits into from
Mar 12, 2021

Conversation

ctb
Copy link
Contributor

@ctb ctb commented Feb 11, 2021

Opens CSV files with newline='' as specified in Python docs (link).

Fixes #1318.

Unfortunately there seems to be no reliable way to test this PR on non-Windows machines because the output is identical with or without newline=''!

Checklist

  • Is it mergeable?
  • make test Did it pass the tests?
  • make coverage Is the new code covered?
  • Did it change the command-line interface? Only additions are allowed
    without a major version increment. Changing file formats also requires a
    major version number increment.
  • Was a spellchecker run on the source code and documentation after
    changes were made?

@codecov
Copy link

codecov bot commented Feb 11, 2021

Codecov Report

Merging #1319 (19f2671) into latest (77467de) will increase coverage by 5.25%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           latest    #1319      +/-   ##
==========================================
+ Coverage   89.15%   94.41%   +5.25%     
==========================================
  Files         123       96      -27     
  Lines       18593    14982    -3611     
  Branches     1432     1433       +1     
==========================================
- Hits        16577    14145    -2432     
+ Misses       1780      601    -1179     
  Partials      236      236              
Flag Coverage Δ
python 94.41% <100.00%> (+<0.01%) ⬆️
rust ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/sourmash/cli/sig/cat.py 100.00% <ø> (ø)
src/sourmash/cli/sig/describe.py 100.00% <ø> (ø)
src/sourmash/cli/sig/split.py 100.00% <ø> (ø)
src/sourmash/cli/sketch/dna.py 100.00% <ø> (ø)
src/sourmash/cli/sketch/protein.py 100.00% <ø> (ø)
src/sourmash/cli/sketch/translate.py 100.00% <ø> (ø)
src/sourmash/commands.py 82.63% <100.00%> (ø)
src/sourmash/lca/command_classify.py 78.75% <100.00%> (ø)
src/sourmash/lca/command_index.py 91.48% <100.00%> (-0.05%) ⬇️
src/sourmash/lca/command_summarize.py 80.80% <100.00%> (ø)
... and 29 more

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 77467de...19f2671. Read the comment docs.

@ctb ctb changed the title [WIP] add proper newline output for csv module [MRG] add proper newline output for csv module Mar 12, 2021
@ctb
Copy link
Contributor Author

ctb commented Mar 12, 2021

Ready for review @bluegenes @luizirber!

@bluegenes
Copy link
Contributor

lgtm, but i don't have a windows machine to test on :)

@ctb
Copy link
Contributor Author

ctb commented Mar 12, 2021

lol, indeed! barring input from @luizirber I think we'll just have to merge & release it and tag in the creator of the original issue.

@ctb ctb merged commit b866935 into latest Mar 12, 2021
@ctb ctb deleted the fix/csv_newline branch March 12, 2021 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSV output too many newlines
2 participants