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

add Type to names as cols #2400

Merged
merged 5 commits into from
Sep 5, 2020
Merged

add Type to names as cols #2400

merged 5 commits into from
Sep 5, 2020

Conversation

bkamins
Copy link
Member

@bkamins bkamins commented Aug 30, 2020

Fixes #2395

Now I understand why I have not added it in the past - AbstractIndex is not aware of column types.

Therefore I propose to add this option to names only. This is useful on its own right. In other places one will have to write something like df[:, names(df, Int)] which is a bit verbose, but not that bad. The benefits are twofold:

  1. This way is explicit and easy to understand when reading the code
  2. I do not have to change dozens of methods (as I would have to add a special method to every existing method that accepts MultiColumnIndex, as we cannot just extend MultiColumnIndex with Type)

@bkamins bkamins added feature non-breaking The proposed change is not breaking labels Aug 30, 2020
@bkamins bkamins added this to the 1.0 milestone Aug 30, 2020
@nalimilan
Copy link
Member

Thanks, looks good. I still think it would be nice to support this everywhere, but indeed it will require a deeper refactoring.

Could you update the categorical/categorical! deprecation to use this?

@bkamins
Copy link
Member Author

bkamins commented Aug 31, 2020

I updated categorical deprecation. When this is merged I will do another update in #2397 and we are cleanly done then.

@pdeffebach
Copy link
Contributor

This is great! Love that we have names to avoid over-complicating indexing.

@bkamins bkamins requested a review from nalimilan September 4, 2020 19:33
@bkamins
Copy link
Member Author

bkamins commented Sep 5, 2020

Now the deprecation of categorical is clean indeed.

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.

Yeah, it's nice that the deprecation is reasonable. That also means many similar operations are now quite easy to do.

@bkamins bkamins merged commit 7fd953f into JuliaData:master Sep 5, 2020
@bkamins bkamins deleted the names_type branch September 5, 2020 12:57
@bkamins
Copy link
Member Author

bkamins commented Sep 5, 2020

Thank you!

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

Successfully merging this pull request may close these issues.

Allow passing type as a column selector of a data frame
3 participants