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

Future of colwise #1595

Closed
bkamins opened this issue Nov 13, 2018 · 3 comments · Fixed by #1778
Closed

Future of colwise #1595

bkamins opened this issue Nov 13, 2018 · 3 comments · Fixed by #1778

Comments

@bkamins
Copy link
Member

bkamins commented Nov 13, 2018

Following #1590 (comment) I would like to discuss here what we should do with colwise function.

Now its basic form colwise(f, df) is the same as map(f, columns(df)), but we have also two additional methods:

  • one producing a matrix when multiple functions are passed as f argument (works only for a data frame);
  • one producing an array of arrays when a single function is applied to a grouped data frame.

How useful do you think those are and is is worth to keep colwise defined or should we deprecate it.

@nalimilan
Copy link
Member

I'm not sure these two methods are very useful. Since they return arrays, they don't include any information about the names of applied functions or about the groups. Anyway this behavior is easy to get using array comprehensions.

Typically, when applying a function on each group, you want the grouping column to be able to interpret the result, so you'd use map(df -> map(f, columns(df)), gd), or map(gd) do f.(columns(df)) end. It could be useful to provide a convenience function to do that, maybe via map(f, columns(gd)) and f.(columns(gd))? The similarity with the DataFrame syntax is appealing.

@bkamins
Copy link
Member Author

bkamins commented Feb 6, 2019

I would deprecate colwise then. OK?

Users can use map or mapcols. I guess in the long run when @nalimilan redesigns GroupedDataFrame API some additional convenience methods for this type can be implemented.

@nalimilan
Copy link
Member

Yes, let's deprecate it!

@bkamins bkamins pinned this issue Feb 6, 2019
@bkamins bkamins unpinned this issue Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants