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

Change @transform on a grouped dataframe to match DataFrames.transform #180

Merged
merged 7 commits into from
Oct 2, 2020

Conversation

pdeffebach
Copy link
Collaborator

also add tests for @select, which already worked after #163

Closes #176 and superscedes #178

@pdeffebach pdeffebach mentioned this pull request Sep 30, 2020
@pdeffebach
Copy link
Collaborator Author

Not sure why the travis build is stalled. But this passes tests locally.

I will merge after 1 approval since it's a small change that has a consensus.

I'm not adding docs yet because the docs need a big re-write and the docus don't currently mention this particular feature.

@bkamins
Copy link
Member

bkamins commented Oct 2, 2020

CI was stalled, because yesterday we had contributors who pushed many small commits to JuliaData - and this is a highly problematic situation (especially if these are commits to DataFrames.jl, whose CI lasts ~1h per commit).

test/grouping.jl Outdated

d = DataFrame(a = [1,1,1,2,2,3,3,1],
b = Any[1,2,3,missing,missing,6.0,5.0,4],
c = CategoricalArray([1,2,3,1,2,3,1,2]))
g = groupby(d, :a)
g = groupby(d, :a)
Copy link
Member

Choose a reason for hiding this comment

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

Indentation seems to be incorrect here and in a few other places.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

indendation looks correct locally. I think github is displaying things weirdly.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like the rest of the file uses tabs. Better use tabs too, and fix the whole file later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should be fixed now. Thanks for finding that.

News.md Outdated
@@ -0,0 +1,5 @@
# DataFramesMeta v0.6 Release Notes
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it more common to call this file NEWS.md in uppercase?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added.

News.md Outdated
Comment on lines 4 to 5
order of rows returned after `DataFrames.transform(gd::GroupedDataFrame, args...)`.
`@select` now supports `GroupedDataFrame` ([#180])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
order of rows returned after `DataFrames.transform(gd::GroupedDataFrame, args...)`.
`@select` now supports `GroupedDataFrame` ([#180])
order of rows returned after `DataFrames.transform(gd::GroupedDataFrame, args...)`.
`@select` now supports `GroupedDataFrame` ([#180])

(I think)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a new bullet for @select, indentation as requested.

@pdeffebach pdeffebach merged commit 0c8e5c9 into JuliaData:master Oct 2, 2020
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 this pull request may close these issues.

Nested transform is broken on master branch
3 participants