Skip to content

Conversation

dshemetov
Copy link
Contributor

@dshemetov dshemetov commented Nov 10, 2020

Fixes a bug where the smoother would drop the index of the Pandas series given to it. The index is restored after smoothing. Base is #477 and contains #437, so should be merged after.

@dshemetov dshemetov requested a review from krivard November 10, 2020 04:19
@krivard krivard marked this pull request as draft November 10, 2020 18:46
@krivard krivard marked this pull request as ready for review November 10, 2020 18:46
@krivard krivard requested review from krivard and removed request for krivard November 10, 2020 18:51
@krivard
Copy link
Contributor

krivard commented Nov 10, 2020

You have some conflicts here, but it's not on any files ostensibly edited by this PR so I don't really understand what's going on.

@dshemetov
Copy link
Contributor Author

That is strange. Any ideas for how to diagnose?

@chinandrew chinandrew changed the base branch from main to aaron-deploy-changehc November 10, 2020 20:51
@chinandrew chinandrew changed the base branch from aaron-deploy-changehc to main November 10, 2020 20:51
@krivard
Copy link
Contributor

krivard commented Nov 10, 2020

Merge with/rebase onto main and see what happens?

@dshemetov
Copy link
Contributor Author

Ok that seemed to fix it?

@dshemetov
Copy link
Contributor Author

I think I can clean up this commit log, let me try rebasing these PRs I made last night.

@dshemetov dshemetov force-pushed the fix_smoother_index_drop branch from 521b0c7 to 6bfa9c1 Compare November 10, 2020 21:43
@dshemetov
Copy link
Contributor Author

Waaay cleaner now: 4 PRs, 4 commits. Thanks Katie, this was a nice learning experience!

@dshemetov dshemetov force-pushed the fix_smoother_index_drop branch from 6bfa9c1 to 647e05d Compare November 10, 2020 22:17
* entire array of nans is handled
* left-padded nans are now ignored
* a few other edge cases
* add tests to match
@dshemetov dshemetov force-pushed the fix_smoother_index_drop branch from 647e05d to 615105c Compare November 11, 2020 00:02
* restore the index after smoothing
* test to match
Copy link
Contributor

@krivard krivard left a comment

Choose a reason for hiding this comment

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

This looks okay to me but I'm a very beginner pandas user so I'm looping in Andrew

@krivard krivard merged commit ad01388 into main Nov 11, 2020
@krivard krivard deleted the fix_smoother_index_drop branch February 9, 2021 19:17
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