Skip to content

fixed tests for min and max#4

Merged
williamma12 merged 2 commits intowilliamma12:fix_testsfrom
osalpekar:mm
Oct 13, 2018
Merged

fixed tests for min and max#4
williamma12 merged 2 commits intowilliamma12:fix_testsfrom
osalpekar:mm

Conversation

@osalpekar
Copy link

Tests for min and max should expect TypeError to be raised for invalid input combinations - this PR adds this to the tests.

with pytest.raises(TypeError):
modin_result = modin_df.max(axis=axis, skipna=skipna, numeric_only=numeric_only)
return
modin_result = modin_df.max(axis=axis, skipna=skipna, numeric_only=numeric_only)
Copy link
Owner

Choose a reason for hiding this comment

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

can this part be moved to the try block so we wont have an empty return statement?

Copy link
Author

Choose a reason for hiding this comment

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

The return is so that execution doesn't move to the next line (which raises an error without being caught, causing the test to fail). Moving the return statement to the try doesn't prevent this so the tests will still show up as failed.

Copy link
Owner

Choose a reason for hiding this comment

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

Not the return statement but the stuff below the return statement into the try block?

Copy link
Author

Choose a reason for hiding this comment

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

ah yeah you're right - updated the PR!

with pytest.raises(TypeError):
modin_result = modin_df.min(axis=axis, skipna=skipna, numeric_only=numeric_only)
return
modin_result = modin_df.min(axis=axis, skipna=skipna, numeric_only=numeric_only)
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above

@williamma12 williamma12 merged this pull request into williamma12:fix_tests Oct 13, 2018
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.

2 participants