Skip to content

fix Error Indexing DafaFrame with a 0-d array #22032

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

Closed
wants to merge 1 commit into from

Conversation

illegalnumbers
Copy link
Contributor

closes #21946

  • [x ] closes #xxxx
  • [x ] tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • [x ] whatsnew entry

df = pd.DataFrame([[1, 2], [3, 4]], columns=['a', 'b'])
ar = np.array(0)
msg = 'Cannot index by location index with a non-integer key'
tm.assert_raises_regex(TypeError, msg,
Copy link
Member

Choose a reason for hiding this comment

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

Can use assertraisesregex as a context here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

type(key) is not tuple)
return (is_list_like(key)
and not (isinstance(key, tuple) and type(key) is not tuple)
and not np.array(key).ndim == 0)
Copy link
Member

Choose a reason for hiding this comment

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

You're really just checking if you have a scalar. Is it possible to use (and potentially modify) is_scalar from pandas.core.dtypes.inference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly! I'll look into that more tomorrow.

@@ -890,6 +890,13 @@ def test_no_reference_cycle(self):
del df
assert wr() is None

def test_zero_index_iloc_raises(self):
df = pd.DataFrame([[1, 2], [3, 4]], columns=['a', 'b'])
Copy link
Member

Choose a reason for hiding this comment

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

Reference issue as a comment below the function def.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do.

@gfyoung gfyoung added Indexing Related to indexing on series/frames, not to indexes themselves Error Reporting Incorrect or improved errors from pandas Numeric Operations Arithmetic, Comparison, and Logical operations labels Jul 23, 2018
@@ -4178,6 +4178,8 @@ def _validate_indexer(self, form, key, kind):
pass
elif is_integer(key):
pass
elif np.array(key).ndim == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

pls don’t do this
you end up materializing the key which could be expensive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea I wasn't really happy with it as is anyways and since it seems to break some tests I'll probably change it in the AM.

@illegalnumbers
Copy link
Contributor Author

Hmm..it seems after reverting my changes and taking a look at my test again that the test still passes. I think I have either a bad test or this was fixed on master before I looked at it? I'm gonna try to build from master and repro in the AM.

@illegalnumbers
Copy link
Contributor Author

Yea confirmed. I checked on master today and we give out the expected error in this instance. TypeError: Cannot index by location index with a non-integer key so I'm gonna close this and comment on the other one as such. Pretty sad :'( but I'll look for something else to contribute with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Error Reporting Incorrect or improved errors from pandas Indexing Related to indexing on series/frames, not to indexes themselves Numeric Operations Arithmetic, Comparison, and Logical operations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error Indexing DafaFrame with a 0-d array
4 participants