-
-
Notifications
You must be signed in to change notification settings - Fork 499
allow string input in dwt_max_level #269
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
Conversation
btw... saw the contributing guidelines link while prepping this, so #268 is working as expected! |
Current coverage is 86.28% (diff: 100%)@@ master #269 diff @@
==========================================
Files 20 20
Lines 3043 3056 +13
Methods 45 45
Messages 0 0
Branches 520 525 +5
==========================================
+ Hits 2624 2637 +13
Misses 368 368
Partials 51 51
|
assert_(pywt.dwt_max_level(32, 'sym5') == 1) | ||
|
||
# string input that is not a discrete wavelet | ||
assert_raises(TypeError, pywt.dwt_max_level, 16, 'mexh') |
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.
This should be ValueError
with an understandable error message ( `filter_len` is not the name of a discrete wavelet
for example.)
Maybe put elif filter_len in wavelist(kind='discrete'):
inside an isinstance(filter_len, string_types):
? I see we don't have string_types
yet; it can be stolen from six
if needed.
LGTM aside from the |
I have included string_types as implemented in six at the top of _dwt.py. Let me know if you would rather have it located in a separate I added another case to catch any other non-integer input, although I used can rebase to a single commit prior to merge |
Had to look this up: numpy/numpy#4547. The May be useful to squash this now before merging. |
…e 2nd argument. update docstring to indicate that a wavelet can be passed in place of the filter length raise ValueError if filter_len < 2 formerly filter_len = 0 or 1 returned 0 while a negative filter_len raised an OverflowError raise informative ValueError on unrecognized string or non-integer input
okay. it should now be ready for merge |
Merged, thanks! |
I noticed that usage of
dwt_max_level
with aWavelet
object as the second argument is fairly common, but is not documented. I updated the docstring and modified the function to also accept a string name as well for convenience.While doing this, I noticed that passing a negative
filter_len
would cause anOverflowError
, while afilter_len
of 0 or 1 would not cause an error, but just return 0. I chose to change the behavior so that anyfilter_len < 2
raises aValueError
.