-
Notifications
You must be signed in to change notification settings - Fork 370
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
Expand LuxGroupby Tests and add bug fixes #287
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good! Just added a comment on hos the test_name_column test could be adjusted. Thanks Kunal!
|
||
def test_name_column(global_var): | ||
df = pd.read_csv("lux/data/car.csv") | ||
new_df = df.rename(columns={"Name": "name"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for this test, can we do a quick check that the values of the "name" column have not all been converted to None values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I added a few more assert statements to check for this case!
These changes looks great, thanks @westernguy2 ! (and thanks to @thyneb19 for helping with the review) |
Overview
This pull request expands the tests for LuxGroupby to test if a LuxDataFrame is pre_aggregated after a groupby. It also patches the edge case of LuxDataFrame having a "name" column among other bug fixes.
Changes
This pull request is mainly for adding tests to LuxGroupby that didn't check if a LuxDataFrame is pre_aggregated after a groupby in certain cases.
In addition, this PR adds the "name" column back to the metadata, which patches the bug described in the above Overview. A test has been added to ensure this doesn't change.
Finally, this PR also patches small bugs relating to groupby and LuxGroupby. For instance, it adds
apply
as one of the methods extended from Pandas Groupby. It also fixes an edge case where the name of a Series was being changed. This is patched by directly editing unnamed columns (named by default as 0) to a blank string.Example Output
The expected behavior should model the expected behavior of Pandas for the different patches described above.