-
Notifications
You must be signed in to change notification settings - Fork 368
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
fix documentation examples #1946
Conversation
docs/src/man/sorting.md
Outdated
@@ -5,7 +5,8 @@ Sorting is a fundamental component of data analysis. Basic sorting is trivial: j | |||
```jldoctest sort | |||
julia> using DataFrames, CSV | |||
|
|||
julia> iris = CSV.read(joinpath(dirname(pathof(DataFrames)), "../docs/src/assets/iris.csv")); | |||
julia> iris = CSV.File(joinpath(dirname(pathof(DataFrames)), "../docs/src/assets/iris.csv")) |> |
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.
Better use the more common syntax DataFrame(...)
? That will avoid confusing newcomers.
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.
fixed (although in my codes I use the File
pattern 😄)
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.
No, I meant using DataFrame(CSV.File(...))
instead of |>
.
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.
OK. Let us stick with CSV.read
with copycols=true
(what I pushed now) as it is also a pattern that is good to show I think.
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 prefer DataFrame(CSV.File(...))
, as it makes it less likely that people will call CSV.read
and forget to pass copycols=true
. Actually I'd rather get rid of CSV.read
.
BTW, there are other occurrences in the docs IIRC, better change all of them at the same time.
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.
OK - done. At least we agree what should be used 😄.
Fixes #1945