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.

Sign up for GitHub

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

Jump to bottom

add Cols support #2495

Merged
merged 3 commits into from
Oct 27, 2020
Merged

add Cols support #2495

merged 3 commits into from
Oct 27, 2020

Conversation

bkamins
Copy link
Member

@bkamins bkamins commented Oct 21, 2020

We have forgotten to add it after DataAPI.jl release

@bkamins bkamins mentioned this pull request Oct 21, 2020
Closed
20 tasks
@bkamins bkamins added this to the 1.0 milestone Oct 21, 2020
@bkamins bkamins added feature non-breaking The proposed change is not breaking labels Oct 21, 2020
@pdeffebach
Copy link
Contributor

Should we deprecate All() in favor of Cols(:)?

@bkamins
Copy link
Member Author

bkamins commented Oct 27, 2020

I was thinking about it. DataAPI.jl supports All so I thought we can leave it, but we can also deprecate it.

@pdeffebach
Copy link
Contributor

x-ref discussions

New discussion in JuliaData/DataAPI.jl#27

nalimilan
nalimilan approved these changes Oct 27, 2020
View reviewed changes
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!

src/other/index.jl Show resolved Hide resolved
@bkamins
Copy link
Member Author

bkamins commented Oct 27, 2020

OK - I understand that we deprecate All in DataAPI.jl. I will make a PR for this.

@bkamins bkamins merged commit d23077a into JuliaData:master Oct 27, 2020
@bkamins bkamins deleted the add_cols branch October 27, 2020 22:39
@bkamins
Copy link
Member Author

bkamins commented Oct 27, 2020

Thank you! After (if) All is removed from DataAPI.jl we need to update DataFrames.jl.

@JuliaRegistrator JuliaRegistrator mentioned this pull request Nov 15, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nalimilan nalimilan nalimilan approved these changes

Assignees
No one assigned
Labels
feature non-breaking The proposed change is not breaking
Projects
None yet
Milestone
1.0
Development

Successfully merging this pull request may close these issues.
3 participants
@bkamins @pdeffebach @nalimilan