-
Notifications
You must be signed in to change notification settings - Fork 367
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
Reintroduce readtable(), writetable() and string-literal macros, keeping them deprecated #1243
Conversation
…ing them deprecated This will ease the transition to CSV, without introducing a dependency on it and without breakage due to behavior changes.
src/deprecated.jl
Outdated
df = DataFrame(A = 1:10) | ||
writetable("output.csv", df) | ||
writetable("output.dat", df, separator = ',', header = false) | ||
writetable("output.dat", df, quotemark = '\', separator = ',') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think quotemark here needs to be escaped, quotemark = '\'
-> quotemark = '\\'
, although I couldn't find any errors in the CI to confirm that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, it must have been there for a very long time. Fixed (I think the correct value is '\''
).
This is just copied and pasted from earlier source code, right? If so, then I don't think we should worry about adding the tests back as well. I don't love the idea of pointing people towards CSV while JuliaData/CSV.jl#109 is unresolved. |
Yeah, that's copy/pasted, except for the place where
We're not going to tag a release immediately anyway, so that leaves time to fix that nasty bug. |
This will ease the transition to CSV, without introducing a dependency on it
and without breakage due to behavior changes.
I tested this locally, but it's not covered by the test suite.
Fixes #1240.