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
Silince UnicodeWarnings in tests #3674
Conversation
@@ -3828,9 +3828,9 @@ def __init__(self, dataset, bw_method=None): | |||
|
|||
if bw_method is None: | |||
pass | |||
elif bw_method == 'scott': |
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.
I think a better fix would be to move this test below the tests for callable
or guard them with not isinstance(bw_method, six.stirng_types)
.
Ensure that bw_method is a string before comparing to strings
Ensure that rotation is string before comparing to string and raise ValueError if rotation could not be converted to float
c90339a
to
43e7831
Compare
43e7831
to
5c12679
Compare
Good points. I initially did the smallest changes needed but we might as well do this probably. I have implemented your suggestions with a few variations. In addition i added a few unittests to get_rotation |
angle = float(rotation) | ||
return angle % 360 | ||
|
||
except (ValueError, TypeError): |
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.
float(None) raises a TypeError
ENH : Silince UnicodeWarnings in tests
|
||
return angle |
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.
Now that I have merged this I noticed that the mod 360 got dropped, was that intentional?
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.
Nope, Missed that, Will fix in a new pr
Fixes warning like the following in the test output:
The changes are somewhat stupid and redundant and the only purpose is to fix the warnings.