-
Notifications
You must be signed in to change notification settings - Fork 369
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
Deprecate length, nrow, and ncol on DataFrames in favor of size. Fixe… #1224
Conversation
+1 for deprecating |
I'd have thought deprecating size would have been more likely at this point. I thought the move to treat DataFrames as matrix-like had fizzled out. length and ncols, for example, would be more of a fit for DataFrames that are conceptually a collection of namedtuples. |
I think there's been more consensus recently to go with I don't see what the big fuss is about deprecating |
I'm not sure what you mean here. Can you elaborate? |
ncol(df::DataFrame) = length(index(df)) | ||
Base.size(df::DataFrame) = ((length(df.columns) > 0 ? length(df.columns[1]) : 0), | ||
length(index(df))) | ||
Base.size(df::DataFrame, i::Int) = i == 1 ? |
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.
Would be cleaner with an if... else
block (with a long-form function definition), the indentation a bit weird here. BTW, could print "$i" in the error message, that's always useful to understand what went wrong.
@quinnj I agree with the issues you brought up about
Leaning solely on a couple (maybe gross) non-base names (like Six in one hand, half dozen in the other -- just wanted to point out that |
FWIW, I like just having
As a result, I'm pretty okay with not supporting any of the options for now. |
|
I haven't done much but lurk and submit one PR, but on this issue I will say this - nrow/ncol are nice, but separate operations. A data frame is not a matrix or an array. It has area, so to speak. size() gets you everything you need in an easy to parse result. From my experience, simpler and complete usually wins in design. Compatibility with other packages and aspects of Julia are, of course, major considerations. I am not sure there is enough concern here about compatibility to disqualify size, no? |
@quinnj How about merging the deprecation of |
Meh, if JuliaLang/julia#22665 settles on |
Doesn't seem |
Changes Unknown when pulling ba396e6 on jq/len into ** on master**. |
Might be late to the party on this, but I'm a fan of
|
Now we have So the decision is do we:
In both cases this PR should probably be closed, but if we want to deprecate them I would open a new PR only with this deprecation. |
I'd rather keep them. Since we've decided data frames are collections of rows, their dimensions are not really symmetric like matrices, so it would be weird to have to use |
Fine with me. |
So close this? |
…s #1200