Skip to content

Add support for savestreaming method, take 2#59

Merged
davidanthoff merged 4 commits intomasterfrom
savestreaming2
Aug 30, 2019
Merged

Add support for savestreaming method, take 2#59
davidanthoff merged 4 commits intomasterfrom
savestreaming2

Conversation

@davidanthoff
Copy link
Copy Markdown
Member

Replaces #53.

The documentation for the API in FileIO.jl is really incomplete, but I think this PR is how it should be done (I tracked down some packages from the person who initially added this feature to FileIO.jl and looked how they are doing it).

The API from a user point of view is:

s = savestreaming(fname, df1)
# OR
s = savestreaming(fname)
# If one doesn't want to write any data at this point.

write(s, df2)
write(s, df2)

close(s)

@lrennels, can you adapt your code to this new API, test whether things work, and then I'll merge this PR here?

@lrennels
Copy link
Copy Markdown

Sure I'll do that this weekend thank you!

@lrennels
Copy link
Copy Markdown

lrennels commented Aug 30, 2019

@davidanthoff I use a dictionary to keep track of the streams, that was previously initialized as

streams::Dict{String, FileIO.Stream}

this now causes a type error, is the following correct now? I could probably also just not type parameterize it.

streams::Dict{String, CSVFiles.CSVFileSaveStream{IOStream}}

@codecov-io
Copy link
Copy Markdown

codecov-io commented Aug 30, 2019

Codecov Report

Merging #59 into master will decrease coverage by 3.26%.
The diff coverage is 72.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
- Coverage     100%   96.73%   -3.27%     
==========================================
  Files           2        2              
  Lines          81       92      +11     
==========================================
+ Hits           81       89       +8     
- Misses          0        3       +3
Impacted Files Coverage Δ
src/csv_writer.jl 94.23% <72.72%> (-5.77%) ⬇️

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 03dc201...76afde8. Read the comment docs.

@lrennels
Copy link
Copy Markdown

@davidanthoff ok all good you can merge and tag!

@davidanthoff davidanthoff merged commit 0775902 into master Aug 30, 2019
@davidanthoff davidanthoff deleted the savestreaming2 branch August 30, 2019 23:31
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.

4 participants