Skip to content
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

[BREAKING] deprecate name => fun in favor of fun => name in describe #2401

Merged
merged 6 commits into from
Sep 8, 2020

Conversation

bkamins
Copy link
Member

@bkamins bkamins commented Aug 31, 2020

Fixes #2385

@bkamins bkamins changed the title deprecate name => fun in favor of fun => name in describe [BREAKING] deprecate name => fun in favor of fun => name in describe Aug 31, 2020
@bkamins bkamins added the breaking The proposed change is breaking. label Aug 31, 2020
@bkamins bkamins added this to the 1.0 milestone Aug 31, 2020
Copy link
Member

@nalimilan nalimilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good with fixed indentation!

src/abstractdataframe/abstractdataframe.jl Show resolved Hide resolved
src/abstractdataframe/abstractdataframe.jl Outdated Show resolved Hide resolved
@bkamins
Copy link
Member Author

bkamins commented Aug 31, 2020

@matthieugomez - what I changed here is what we all agree to change.

Now - do you have strong preference for your proposal? If yes, just to make sure, you envision the signature to be:

describe(df, cols=:; stats=[:mean, :min, :median, :max, :nmissing, :eltype])

where stats is a single stat or a vector of statistics to calculate?

(if we are breaking things I would not oppose this that much, but I want to make sure that your proposal has the general support)

@matthieugomez
Copy link
Contributor

matthieugomez commented Sep 1, 2020 via email

@bkamins
Copy link
Member Author

bkamins commented Sep 1, 2020

@nalimilan - do you have any preference here?

@nalimilan
Copy link
Member

I don't have a strong opinion. At least what is clear is that functions which take a cols (positional) argument only allow a single one in general, but functions that take source => fun => dest allow varargs. So in that sense the current situation is consistent.

@bkamins
Copy link
Member Author

bkamins commented Sep 1, 2020

Thank you for considering it with such care. Given all pros and cons I am leaning to leave the PR as it stands now.

@bkamins
Copy link
Member Author

bkamins commented Sep 4, 2020

@nalimilan - so do we go for this or wait a bit for more feedback (I feel all has been said already).

@nalimilan
Copy link
Member

Yeah, let's go with this since there's no perfect solution.

NEWS.md Outdated Show resolved Hide resolved
src/abstractdataframe/abstractdataframe.jl Outdated Show resolved Hide resolved
src/abstractdataframe/abstractdataframe.jl Outdated Show resolved Hide resolved
test/dataframe.jl Show resolved Hide resolved
bkamins and others added 2 commits September 7, 2020 17:49
Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>
@bkamins bkamins merged commit 9f90361 into JuliaData:master Sep 8, 2020
@bkamins bkamins deleted the describe_deprecate_pair_order branch September 8, 2020 13:11
@bkamins
Copy link
Member Author

bkamins commented Sep 8, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking The proposed change is breaking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

describe doesn't follow the source => function => name convention
3 participants