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

FIX-#31: Fix test_nans and add mixed sort test #32

Merged
merged 2 commits into from
Apr 8, 2021

Conversation

valerieyip
Copy link

@valerieyip valerieyip commented Mar 19, 2021

Resolves #31 test_nans was not working because the column name was an integer and grid.py concatenates a string to column names to ensure unique column names. Fix by casting to string.

Added a test_mixed_type_sort with mixed data types such as np.nan as well as sorting a column with an integer column name.

@valerieyip valerieyip changed the title Fix-#31 FIX-#31: Fix test_nans and add mixed sort test Mar 19, 2021
@richardlin047 richardlin047 self-requested a review March 24, 2021 02:29
Copy link
Collaborator

@richardlin047 richardlin047 left a comment

Choose a reason for hiding this comment

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

Just needs some minor changes.

modin_spreadsheet/tests/test_grid.py Outdated Show resolved Hide resolved
modin_spreadsheet/tests/test_grid.py Outdated Show resolved Hide resolved
modin_spreadsheet/tests/test_grid.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@richardlin047 richardlin047 left a comment

Choose a reason for hiding this comment

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

@valerieyip LGTM, just need to rebase master and fix the conflict in modin_spreadsheet/tests/test_grid.py

…d data columns and row values

Signed-off-by: Valerie Yip <valerieyip@berkeley.edu>
…readability

Signed-off-by: Valerie Yip <valerieyip@berkeley.edu>
@richardlin047 richardlin047 merged commit abb2fda into modin-project:master Apr 8, 2021
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.

Fix test_nans()
2 participants