Skip to content

Conversation

jbrockmendel
Copy link
Member

Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

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

LGTM.

Is this a behavior we want to deprecate and change in the future? Or was the original change unintentional?

@TomAugspurger TomAugspurger added this to the 1.0.1 milestone Jan 31, 2020
@TomAugspurger TomAugspurger added the Indexing Related to indexing on series/frames, not to indexes themselves label Jan 31, 2020
@jbrockmendel
Copy link
Member Author

The change that this reverts was unintentional (was a code-cleanup intended to remove accessing a private loc method). That said, I do think we should deprecate it.

@jorisvandenbossche jorisvandenbossche changed the title REG: DataFrame.__setitem__(slice, val) is positional REGR: DataFrame.__setitem__(slice, val) is positional Jan 31, 2020
Co-Authored-By: Joris Van den Bossche <[email protected]>
@TomAugspurger
Copy link
Contributor

OK, will need a followup issue for that deprecation if you want to push for it.

Let's give @jreback a chance to look, but this should be good to go.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm, surprised we didn’t have a test for this; may want to add some additional cases (eg closed slice in right, open on left and closed on right) - basically the other slice cases


assert (float_frame["C"] == 4).all()

def test_setitem_slice_position(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

@jbrockmendel if you can also test (df[:4]) for example (can just target master is ok)

@jreback jreback merged commit 0ca9ddd into pandas-dev:master Feb 1, 2020
@TomAugspurger TomAugspurger mentioned this pull request Feb 1, 2020
@jbrockmendel jbrockmendel deleted the fix-31469 branch February 1, 2020 16:13
@jorisvandenbossche
Copy link
Member

This still needs a backport?

@jorisvandenbossche
Copy link
Member

@meeseeksdev backport to 1.0.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Indexing Related to indexing on series/frames, not to indexes themselves
Projects
None yet
Development

Successfully merging this pull request may close these issues.

REGR: __setitem__ with integer slices on Int/RangeIndex is broken (label instead of positional)
4 participants