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

kendalltau function now returns a nan tuple if empty arrays used as para... #3885

Merged
merged 3 commits into from Aug 19, 2014
Merged

Conversation

ctokheim
Copy link
Contributor

...meters instead of raising a runtime exception. Previously, empty arrays would have cause a maximum recursion depth exceeded error in mergesort if the provided arrays were empty (issue #3884).

…arameters instead of raising a runtime exception
@argriffing
Copy link
Contributor

A regression test would be good.

@ctokheim
Copy link
Contributor Author

Should I do something? Or is that what Travis CI is for?

@argriffing
Copy link
Contributor

TravisCI will run the tests to try catching regressions, but I was thinking that adding a new test would be appropriate. For example, a test could be added that would have caused a RuntimeError but no longer raises the exception after your change.

@ctokheim
Copy link
Contributor Author

Ah, that makes sense.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 54704c2 on ctokheim:master into * on scipy:master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 061a5a8 on ctokheim:master into * on scipy:master*.

@@ -633,6 +633,9 @@ def test_kendalltau():
assert_(np.all(np.isnan(stats.kendalltau([2,0,2], [2,2,2]))))
assert_(np.all(np.isnan(stats.kendalltau([2,2,2], [2,0,2]))))

# empty arrays provided as input
assert_(np.all(np.isnan(stats.kendalltau([], []))))
Copy link
Member

Choose a reason for hiding this comment

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

assert_equal has code to handle nan, so this can be simplified to assert_equal(stats.kendalltau([], []), (np.nan, np.nan)).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I changed the checking for both the tie case and empty array case to use assert_equal.

@rgommers rgommers added this to the 0.15.0 milestone Aug 19, 2014
rgommers added a commit that referenced this pull request Aug 19, 2014
kendalltau function now returns a nan tuple if empty arrays used as para...
@rgommers rgommers merged commit b47877d into scipy:master Aug 19, 2014
@rgommers
Copy link
Member

Looks good, merging. Thanks @ctokheim.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants