-
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
Do we need CSV.read? #2309
Comments
|
Most of the time you want to omit The problem with doing a type piracy over @quinnj - maybe the solution is to define
In such a case |
This isn't accurate as of the 0.7 CSV.jl release; with 0.7, you get back fully mutable arrays. |
Ah - it is plainly wrong :(. I have not tested this enough yet. So now indeed using |
I don't see the need to privledge CSV.jl over any other Table type. Its not like its much extra typing, CSV has a very short package name. |
CSV.read(file; kwargs...) = CSV.File(file; kwargs...) The idea is that |
I think having both CSV.read(::Type{DataFrame}, file; kwargs...) = DataFrame!(CSV.File(file; kwargs)) |
to match |
Very torn on this one - on the one hand I'm fully in the camp that says On balance it therefore seems to me that the |
I'm also kinda torn. Coming from pandas, I initially thought it was sort of absurd to remove/change I've come around though, @bkamins has convinced me how valuable a concise API is. That said
|
Fixes JuliaData/DataFrames.jl#2309. After much discussion, people really like the `CSV.read` function call naming, so it was decided to keep it around, while still allowing a break from DataFrames.jl as a dependency. I also realized that `CSV.read` does indeed provide one bit of value/functionality: we can wrap `CSV.File` in `Tables.CopiedColumns`. What this means is that columnar sinks can safely use the columns passed without needing to make copies; i.e. they can assume ownership of the columns. In `CSV.read`, the user is essentially saying, "I want to make a `CSV.File` and pass it directly to `sink`" which also implies that `CSV.File` doesn't need to "own" its own columns. The only question left in my mind is, with the 1.0 release, what to do with `CSV.read(file)` when no `sink` argument is passed. Suggestions have included just returning a `CSV.File`, since it's a valid table anyway. Or allowing DataFrames.jl to define the no-sink-arg version and return a `DataFrame`; that one's a bit awkward as a form of "blessed" type piracy, but could also be useful for users as convenience (they just have to remember to do `using DataFrames` before trying to use it). The other awkward part is that we're currently warning users of the deprecation and that they should explicitly spell out `DataFrame` whereas we might not actually require that.
PR up! JuliaData/CSV.jl#687. Feel free to comment there on implementation, or here on any other questions/concerns. |
FYI, |
It has been defined in CSV.jl but now is deprecated. Do we need this function or:
is enough.
Please comment here to have a record of the dicussion.
The text was updated successfully, but these errors were encountered: