Skip to content

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