Skip to content

CLN: remove nested error handling #27792

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

Merged
merged 7 commits into from
Aug 7, 2019
Merged

Conversation

jbrockmendel
Copy link
Member

The removed code is not covered in the test suite, so this makes for a nice little simplification. Moreover, since the condition above on 1369-1373 is an ugly one, de-nesting this function will give us a good shot and further simplifying this code.

That said, this could be considered an API change, since now the "errors" kwarg doesn't actually do anything.

@gfyoung gfyoung added the Clean label Aug 7, 2019
@gfyoung
Copy link
Member

gfyoung commented Aug 7, 2019

Some stuff to unpack here with your PR description!

The removed code is not covered in the test suite, so this makes for a nice little simplification.

When did that become justification for removing code? 🙂

That said, this could be considered an API change, since now the "errors" kwarg doesn't actually do anything.

Hmm...I'm always a little on the fence when it comes to simplifications that have API change side-effects, not matter how small. I guess it's a judgment call, but I do appreciate the simplification.

@jbrockmendel
Copy link
Member Author

When did that become justification for removing code? 🙂

Many chunks of code were previously intended for Panel. Non-covered code isn't necessarily removable, but it makes for a good place to look for un-needed code.

I guess it's a judgment call, but I do appreciate the simplification.

Yah, the real upside is that this makes it much simpler to remove _try_coerce_args, which would complete an ongoing simplification goal.

@jreback jreback added this to the 1.0 milestone Aug 7, 2019
@jreback jreback merged commit 3581073 into pandas-dev:master Aug 7, 2019
@jreback
Copy link
Contributor

jreback commented Aug 7, 2019

thanks @jbrockmendel

@jbrockmendel jbrockmendel deleted the erraise branch August 7, 2019 17:18
quintusdias pushed a commit to quintusdias/pandas_dev that referenced this pull request Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants