Small improvements to missing value support #1298
Conversation
cjprybol
force-pushed
the
cjp/similar
branch
2 times, most recently
from
734d44e
to
55665d3
Compare
| that is different than the number of rows present in `df`. | ||
| """ | ||
| function Base.similar(df::AbstractDataFrame, rows::Integer = size(df, 1)) | ||
| @assert rows >= 0 "the number of rows must be positive" |
There was a problem hiding this comment.
Throw an ArgumentError instead.
| end | ||
|
|
||
| """ | ||
| similar_missing(df::DataFrame[, rows::Int]) |
There was a problem hiding this comment.
I'd prefer if we avoided adding this function until we're completely certain it's needed. People can use allowmissing!, and hopefully the compiler will be able to make this efficient at some point.
src/dataframe/dataframe.jl
Outdated
|
|
||
| Convert a single column of a DataFrame from element-type `T` to |
There was a problem hiding this comment.
`DataFrame` and "element type".
I think you could merge the two docstrings into a single one, with one line for each signature (or maybe three, with a line for the single-argument method). Attach it to function allowmissing! end.
src/dataframe/dataframe.jl
Outdated
|
|
||
| Convert columns of a DataFrame in-place from element-type `T` to | ||
| `Union{T, Missing}`. Defaults to converting all columns of the DataFrame, | ||
| although users can manually specify a subset of columns to convert using the |
There was a problem hiding this comment.
I don't really like talking about "users" here, it's weird for the "user" who is reading the docstring. :-)
test/dataframe.jl
Outdated
| b = CategoricalArray(["foo"]), | ||
| c = [0.0], | ||
| d = CategoricalArray([0.0])) | ||
| @test typeof.(df.columns) == typeof.(similar(df).columns) |
There was a problem hiding this comment.
Also test the size of the result, and passing a valid number or rows different from the original?
test/dataframe.jl
Outdated
| @test missingdf ≅ similar(df, 2) | ||
| b = missings(String, 2), | ||
| c = CategoricalArray{Union{Float64, Missing}}(2)) | ||
| # @test missingdf ≅ similar(df, 2) |
There was a problem hiding this comment.
There was a problem hiding this comment.
Better leave a comment pointing to that issue, and in the meantime check the types and size of the result.
src/dataframe/dataframe.jl
Outdated
| Convert all columns of a `DataFrame` from element type `T` to | ||
| `Union{T, Missing}` to support missing values. | ||
|
|
||
| allowmissing!(df::DataFrame, col::ColumnIndex) |
There was a problem hiding this comment.
ColumnIndex is not exported, so I guess we should use Union{Integer, Symbol}. Not sure what we do in other docstrings.
- change behavior of `similar` on DataFrames to not auto-enable missing value support and instead use same column eltypes as parent df - add docstrings for `similar` and `allowmissing!` - change behavior of `allowmissing!` on CategoricalArrays to preserve correct Array type (which is a CategoricalArray) - add `similar` to docs
|
Thanks for tagging a new release of Missings @nalimilan! Closing to re-run tests |
similaron DataFrames to not auto-enable missingvalue support and instead use same column eltypes as parent df
similarandallowmissing!allowmissing!on CategoricalArrays to preservecorrect Array type (which is a CategoricalArray)
similarto docsShould fix #1294 and #1291, although it does not address the
append!ideas presented in #1291.