Update smoother nan handling #477
Conversation
| if self.impute_method not in valid_impute_methods: | ||
| raise ValueError("Invalid impute method given.") | ||
| if self.window_length <= 1: | ||
| raise ValueError("Window length is too short.") |
There was a problem hiding this comment.
add test for this error
dshemetov
force-pushed
the
fix_smoother_nan_handling
branch
4 times, most recently
from
4421f28 to
6d2ef32
Compare
* entire array of nans is handled * left-padded nans are now ignored * a few other edge cases * add tests to match
dshemetov
force-pushed
the
fix_smoother_nan_handling
branch
from
6d2ef32 to
4ad4c39
Compare
chinandrew
left a comment
chinandrew
left a comment
There was a problem hiding this comment.
theres a whitespace hanging around, otherwise 
Co-authored-by: chinandrew <[email protected]>
|
fixed! do you know if there's a way to merge commits without having to reset and force push every time? |
If the commits were previously pushed to the remote branch, I think you'll have to force push after squashing. |
|
I hope github doesn't send emails every time I do that? |
I get github emails for all new commits on every PR, so my signal to noise ratio is pretty bad already. I just use a filter for the ones that I'm reviewing or am mentioned in and it works alright. |
|
Oh no. I'm sorry for everyone's inboxes. Chains of dependent PRs: not even once. |
It seemed that there were many cases in JHU tests that required the smoother to be able to act on an array full of nans, so instead of throwing an error, I improved things to be more graceful.
Basically handles the following cases:
Its base is #476, so should be merged after.