-
-
Notifications
You must be signed in to change notification settings - Fork 156
allow np.uint64 to be used in indexing. Support numpy 1.24.1 #510
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
Conversation
| pd.DataFrame, | ||
| ) | ||
| with pytest.warns(np.VisibleDeprecationWarning): | ||
| if Version(np.__version__) <= Version("1.23.5"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be fine removing tests for older versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove this test once the bug is fixed in pandas. There is a PR for that now at pandas-dev/pandas#50682
| df = pd.DataFrame(dict(x=[1, 2, 3]), index=np.array([10, 20, 30], dtype="uint64")) | ||
|
|
||
| def get_NDArray(df: pd.DataFrame, key: npt.NDArray[np.uint64]) -> pd.DataFrame: | ||
| df2 = df.loc[key] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't any np.NDArray work (not just integer) as long as the DataFrame index is of the same type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could probably only enforce a tight dtype match if index was generic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't any
np.NDArraywork (not just integer) as long as the DataFrame index is of the same type?
Probably, but since we can't track the dtype of an Index in a DataFrame, I'm limiting this for now to the issue that was reported. I think most people use arrays of int or arrays of str (which we probably could add), but I'd rather be incremental in adding support for all the possible types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could probably only enforce a tight dtype match if index was generic
If we knew the dtype of the underlying index, but we don't know that.
|
Thanks @Dr-Irv ! |
test_frame.py:test_npint_loc_indexer()Turns out that
np.uint64is not a subclass ofnp.int64, so changed the indexing type to benp.integer, which is the parent class ofnp.int64andnp.uint64. Error only occurred with numpy 1.24.1, so modified things to allow us to test with that version.