Skip to content
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

REGR: Series access with Index of tuples/frozenset #36147

Merged
merged 12 commits into from
Sep 12, 2020

Conversation

rhshadrach
Copy link
Member

@rhshadrach rhshadrach added this to the 1.1.2 milestone Sep 5, 2020
@rhshadrach rhshadrach added Indexing Related to indexing on series/frames, not to indexes themselves Regression Functionality that used to work in a prior pandas version Series Series data structure labels Sep 5, 2020
result = self._get_value(key)

return result
if isinstance(self.index, MultiIndex):
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of doing the if first, can you do it in the keyerror itself e.g.

try:
    result = self._get_value(key)
    return result
except KeyError:
    if isinstance(self.index, MultiIndex):
        return self._get_values_tuple(key)

of course maybe we should actually do this inside _get_value

Copy link
Member Author

@rhshadrach rhshadrach Sep 7, 2020

Choose a reason for hiding this comment

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

Since _get_value is called from 4 places, I'll look into doing this for 1.2 in a followup.

@rhshadrach
Copy link
Member Author

Thanks @jreback, I reworked the logic. This patch now retains much more of the original behavior.

When one tries to find a tuple key in a Series with an Index, originally you got the message Can only tuple-index with a MultiIndex. As this is no longer true, I've changed this to key of type tuple not found and not a MultiIndex.

@jreback
Copy link
Contributor

jreback commented Sep 8, 2020

@simonjayhawkins this is ok to merge on green if you haven't tagged (otherwise can move to 1.1.3)

@simonjayhawkins
Copy link
Member

@simonjayhawkins this is ok to merge on green if you haven't tagged (otherwise can move to 1.1.3)

yep

@simonjayhawkins
Copy link
Member

restarting ci. known failure

=========================== short test summary info ===========================
FAILED pandas/tests/plotting/test_groupby.py::TestDataFrameGroupByPlots::test_series_groupby_plotting_nominally_works
= 1 failed, 74726 passed, 1786 skipped, 1130 xfailed, 6 xpassed, 67 warnings in 1183.92s (0:19:43) =

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

same test failed. may need further investigation

@simonjayhawkins simonjayhawkins mentioned this pull request Sep 8, 2020
@jreback
Copy link
Contributor

jreback commented Sep 8, 2020

ok let's defer this one

@jreback jreback modified the milestones: 1.1.2, 1.1.3 Sep 8, 2020
doc/source/whatsnew/v1.1.2.rst Outdated Show resolved Hide resolved
@jbrockmendel
Copy link
Member

LGTM whatsnew-placement notwithstanding

@rhshadrach
Copy link
Member Author

Failure is different this time.

=========================== short test summary info ===========================
FAILED pandas/tests/io/test_parquet.py::TestParquetPyArrow::test_s3_roundtrip_for_dir[partition_col0]
= 1 failed, 74674 passed, 1905 skipped, 1108 xfailed, 6 xpassed, 284 warnings in 1178.42s (0:19:38) =

@simonjayhawkins
Copy link
Member

Failure is different this time.

I think this one is affecting all PRs

@rhshadrach
Copy link
Member Author

@jreback Same unrelated parquet failure; the groupby plots failure no longer occurs.

pandas/core/series.py Outdated Show resolved Hide resolved
@rhshadrach
Copy link
Member Author

Thanks for the review @jbrockmendel, changes made and checks pass.

@jreback jreback merged commit 822dc6f into pandas-dev:master Sep 12, 2020
@jreback
Copy link
Contributor

jreback commented Sep 12, 2020

thanks @rhshadrach for the followon's can open an issue if you want so we can track.

@rhshadrach rhshadrach deleted the tuple_regression branch September 13, 2020 11:58
@simonjayhawkins
Copy link
Member

@meeseeksdev backport 1.1.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 Regression Functionality that used to work in a prior pandas version Series Series data structure
Projects
None yet
4 participants