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.

Sign up for GitHub

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

Jump to bottom

DEPR: Index.asi8 #37877

Merged
merged 9 commits into from Nov 19, 2020
Merged

DEPR: Index.asi8 #37877

merged 9 commits into from Nov 19, 2020
+55 −12

Conversation

jbrockmendel
Copy link
Member

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

@jbrockmendel jbrockmendel changed the title Depr asi8 DEPR: Index.asi8 Nov 15, 2020
jreback
jreback requested changes Nov 15, 2020
View reviewed changes
pandas/core/indexes/base.py
@@ -415,6 +415,11 @@ def asi8(self):
ndarray
An ndarray with int64 dtype.
"""
warnings.warn(
"Index.asi8 is deprecated and will be removed in a future version",
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add that the replacement is .astype('i8')

Copy link
Member Author

Choose a reason for hiding this comment

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

trouble is thats not really correct for the Index classes we're deprecating it for. For these classes .asi8 returns None

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh ok. are we deprecating for DTI and cousins? (I think we should do that as well to be consistent).

Copy link
Member Author

Choose a reason for hiding this comment

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

ill take a look; that might be pretty invasive

Copy link
Contributor

Choose a reason for hiding this comment

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

ok i think you should make this an internal method and deprecate the user facing one (even though we use it internally in the many parts of pandas)

pandas/core/indexes/base.py
result = np.array(self)
if needs_i8_conversion(self.dtype):
# TODO: these do not match the underlying EA argsort methods GH#37863
return self.asi8.argsort(*args, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you calling .asi8 here?

Copy link
Member Author

Choose a reason for hiding this comment

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

comment just above #37863 a handful of tests fail if we pass through to the backing EA.argsort

Copy link
Contributor

Choose a reason for hiding this comment

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

won't these show the deprecation warning?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, because we're not deprecating it for the datetimelike classes

pandas/core/tools/numeric.py
values = arg.asi8
if values is None:
if needs_i8_conversion(arg.dtype):
values = arg.asi8
Copy link
Contributor

Choose a reason for hiding this comment

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

aren't you deprecating this?

Copy link
Member Author

Choose a reason for hiding this comment

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

only deprecating the one in pd.Index that returns None

@jreback jreback added Deprecate Functionality to remove in pandas Index Related to the Index class or subclasses labels Nov 15, 2020
@jreback jreback mentioned this pull request Nov 15, 2020
Closed
@jreback jreback added this to the 1.2 milestone Nov 17, 2020
jorisvandenbossche
jorisvandenbossche requested changes Nov 17, 2020
View reviewed changes
doc/source/whatsnew/v1.2.0.rst Outdated
@@ -457,7 +457,8 @@ Deprecations
- :class:`Index` methods ``&``, ``|``, and ``^`` behaving as the set operations :meth:`Index.intersection`, :meth:`Index.union`, and :meth:`Index.symmetric_difference`, respectively, are deprecated and in the future will behave as pointwise boolean operations matching :class:`Series` behavior. Use the named set methods instead (:issue:`36758`)
- :meth:`Categorical.is_dtype_equal` and :meth:`CategoricalIndex.is_dtype_equal` are deprecated, will be removed in a future version (:issue:`37545`)
- :meth:`Series.slice_shift` and :meth:`DataFrame.slice_shift` are deprecated, use :meth:`Series.shift` or :meth:`DataFrame.shift` instead (:issue:`37601`)
- Partial slicing on unordered :class:`DatetimeIndexes` with keys, which are not in Index is deprecated and will be removed in a future version (:issue:`18531`)
- Partial slicing on unordered :class:`DatetimeIndex` with keys, which are not in Index is deprecated and will be removed in a future version (:issue:`18531`)
- Deprecated :meth:`Index.asi8` (:issue:`37877`)
Copy link
Member

Choose a reason for hiding this comment

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

Can you clarify this deprecation is only for the non-datetimelike Index subclasses for which the attribute currently returns None?

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea, will update

Copy link
Member Author

Choose a reason for hiding this comment

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

updated + greenish

@jorisvandenbossche
Copy link
Member

(whatsnew conflict)

@jreback
Copy link
Contributor

jreback commented Nov 18, 2020

ok resolved, will merge on greenish

@jreback jreback merged commit d5b2094 into pandas-dev:master Nov 19, 2020
@jreback
Copy link
Contributor

jreback commented Nov 19, 2020

thanks @jbrockmendel

@jbrockmendel jbrockmendel deleted the depr-asi8 branch November 19, 2020 00:27
@mzeitlin11 mzeitlin11 mentioned this pull request Dec 22, 2020
Merged
4 tasks
@itholic itholic mentioned this pull request Sep 14, 2023
Closed
dongjoon-hyun pushed a commit to apache/spark that referenced this pull request Sep 16, 2023
@itholic @dongjoon-hyun
[SPARK-45164][PS] Remove deprecated Index APIs
69ba1bd 
### What changes were proposed in this pull request?

This PR proposes to remove deprecated `Index` APIs from Pandas API on Spark.

### Why are the changes needed?

To follow the behavior of the latest Pandas.

See pandas-dev/pandas#37877 for `Index.asi8`

See pandas-dev/pandas#42113 for `Index.is_type_compatible`

### Does this PR introduce _any_ user-facing change?

`Index.asi8`, `Index.is_type_compatible` is removed. `Index.astype` and `Index.isin` can be used instead respectively.

### How was this patch tested?

The existing CI should pass.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #42926 from itholic/SPARK-45164.

Authored-by: Haejoon Lee <haejoon.lee@databricks.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

@jorisvandenbossche jorisvandenbossche jorisvandenbossche requested changes

Assignees
No one assigned
Labels
Deprecate Functionality to remove in pandas Index Related to the Index class or subclasses
Projects
None yet
Milestone
1.2
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jbrockmendel @jorisvandenbossche @jreback