BUG: Various inconsistencies in DataFrame getitem/setitem behavior (fixes #2765)

[stephenwlin]

see #2765

Note, to address " getitem does not reindex boolean Series key but setitem does ", I have changed the getitem behavior rather than the setitem behavior because it is more consistent with other indexing behavior and does not break any tests...just in case, I have inserted a warning about this happening (which only triggers when a reindex is actually required)

Also, in addition to resolving the bugs, I have refactored the code to make getitem and setitem code paths as symmetric as possible, in order to help prevent more issues in the future.

[ghost]

Last time I tumbled down the spaghetti stack of __getitem__ I encountered a helpful
"here be performance dragons" warning comment left somewhere by wesm for the potential
yet-to-be scarred contributor.

So, just a hunch, but did you check for perf regressions for this vs. master?
(using test_perf.sh is probably most convenient).

[stephenwlin]

ok, will take a look

[stephenwlin]

ok, only one test consistently affected: after a tweak, things are good now (test_perf.sh gives ratios that look basically normally distributed around 1.0, with no single test consistently doing worse across multiple runs)

[ghost]

ok, good.

If you're changing behaviour of __getitem__ and __setitem__, it's a good idea
to reify the desired behaviour with some new tests, since there's obviously
no coverage there right now and It'll make it easier to review as well.

[stephenwlin]

yeah, already added test coverage

[ghost]

Sorry, I missed that when looking.

[stephenwlin]

found a leftover print statement in my test code...decided to consolidated the merges and made a new PR at #2776 rather than spamming the commit log

[ghost]

Github allows you to do a forced push to the PR branch, in which case, the current state of the branch
replaces the original PR commits. No need for a new PR (next time).

[stephenwlin]

Ahh, wasn't sure if Github handled non-ff pushes to PR branches well. What does the PR show on the website when you do that? I guess I'll find out next time.

[ghost]

It throws away the old commits and shows just the new state of the branch.

[stephenwlin]

ok, good to know