DEPR: deprecate Index.is_categorical #50225

ShisuiUzumaki · 2022-12-13T09:20:14Z

progress towards API: Deprecate Index.is_(boolean|categorical|integer|floating|numeric|object|interval) #50042
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest doc/source/whatsnew/v2.0.0.rst file if fixing a bug or adding a new feature.

ShisuiUzumaki · 2022-12-13T11:13:24Z

@topper-123 Please have a look. Although I am not sure why so many of the checks are failing. Could you tell me if you know?

topper-123 · 2022-12-13T12:45:45Z

You have to either

look through the test logs to see what errors the are reporting
run the test suite yourself

The second option is preferable, but is a bit more demanding, e.g. you will have to learn how to compile the pandas code and more. However I recommend doing that, as there is valuable knowledge in knowing that. see here

If you fix this by looking through the logs, press "details" on the right of a test and go through the log text to see if you find the issue. I typically do that by pressing "view raw logs" and searching for the text string "FAILED". You can try that. In this case here are some log texts from your PR, see if this helps:

2022-12-13T09:59:39.0966248Z FAILED pandas/tests/indexes/test_base.py::TestIndex::test_is_categorical_is_deprecated - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0966954Z FAILED pandas/tests/indexes/test_base.py::TestMixedIntIndex::test_is_categorical_is_deprecated - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0967709Z FAILED pandas/tests/indexes/categorical/test_category.py::TestCategoricalIndex::test_is_categorical_is_deprecated - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0968475Z FAILED pandas/tests/indexes/datetimes/test_datetimelike.py::TestDatetimeIndex::test_is_categorical_is_deprecated - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0969201Z FAILED pandas/tests/indexes/interval/test_base.py::TestBase::test_is_categorical_is_deprecated - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0969959Z FAILED pandas/tests/indexes/numeric/test_numeric.py::TestFloatNumericIndex::test_is_categorical_is_deprecated[float64] - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0970741Z FAILED pandas/tests/indexes/numeric/test_numeric.py::TestFloatNumericIndex::test_is_categorical_is_deprecated[float32] - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0971498Z FAILED pandas/tests/indexes/numeric/test_numeric.py::TestIntNumericIndex::test_is_categorical_is_deprecated[int64] - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0972243Z FAILED pandas/tests/indexes/numeric/test_numeric.py::TestIntNumericIndex::test_is_categorical_is_deprecated[int32] - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0972975Z FAILED pandas/tests/indexes/numeric/test_numeric.py::TestIntNumericIndex::test_is_categorical_is_deprecated[int16] - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0973710Z FAILED pandas/tests/indexes/numeric/test_numeric.py::TestIntNumericIndex::test_is_categorical_is_deprecated[int8] - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0974462Z FAILED pandas/tests/indexes/numeric/test_numeric.py::TestUIntNumericIndex::test_is_categorical_is_deprecated[uint64] - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0975245Z FAILED pandas/tests/indexes/period/test_period.py::TestPeriodIndex::test_is_categorical_is_deprecated - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0975963Z FAILED pandas/tests/indexes/ranges/test_range.py::TestRangeIndex::test_is_categorical_is_deprecated - AssertionError: Did not see expected warning of class 'FutureWarning'
2022-12-13T09:59:39.0976709Z FAILED pandas/tests/indexes/timedeltas/test_timedelta.py::TestTimedeltaIndex::test_is_categorical_is_deprecated - AssertionError: Did not see expected warning of class 'FutureWarning'

topper-123 · 2022-12-13T19:33:08Z

Great.

I do see you have a few unrelated commit among yours. You should get rid of those.

ShisuiUzumaki · 2022-12-14T03:11:59Z

Great.

I do see you have a few unrelated commit among yours. You should get rid of those.

@topper-123 Is there a clean method of deleting unrelated commits?

Also, I think all checks are passed but one

topper-123 · 2022-12-14T08:16:16Z

Is there a clean method of deleting unrelated commits?

it looks like they’re gone now, so that’s good.

topper-123 · 2022-12-14T08:19:47Z

All the renaming under the “Returns” sections seem unrelated and not needed for this PR, is that correct? If yes, can you get tid of them.

ShisuiUzumaki · 2022-12-14T09:26:10Z

All the renaming under the “Returns” sections seem unrelated and not needed for this PR, is that correct? If yes, can you get tid of them.

Return section?

topper-123 · 2022-12-14T09:30:54Z

In the doc strings, where it starts with

Returns
-------

ShisuiUzumaki · 2022-12-14T09:48:22Z

I am a bit confused which file/method are you referring to. Did I make any changes in that section as well?

topper-123 · 2022-12-14T09:55:59Z

All the instances where you chaange the text below the "Returns" section.

for example in pandas/core/indexes/base.py you change:

Returns
-------
Index

to

Returns
-------
union : Index

Can you roll back all changes like this.

ShisuiUzumaki · 2022-12-14T09:58:43Z

All the instances where you chaange the text below the "Returns" section.

for example in pandas/core/indexes/base.py you change:
Returns
-------
Index
to
Returns
-------
union : Index
Can you roll back all changes like this.

Yes, I just found that out through git history, I am on to it

ShisuiUzumaki · 2022-12-14T10:45:58Z

@topper-123 Now there is a test failure

File "/home/runner/micromamba/envs/test/lib/python3.8/site-packages/requests/models.py", line 10[21](https://github.com/pandas-dev/pandas/actions/runs/3693694966/jobs/6253988365#step:5:22), in raise_for_status raise HTTPError(http_error_msg, response=self) requests.exceptions.HTTPError: 403 Client Error: rate limit exceeded for url: https://api.github.com/users/sinhrks

topper-123 · 2022-12-14T11:17:06Z

that is unrelated to your PR, but has to do with the CI. I've restarted it, should hopefully work now.

I see there is still issues of the type I mentioned above, e.g. new_index : %(klass)s has replaced %(klass)s and many others.

ShisuiUzumaki · 2022-12-14T14:19:47Z

that is unrelated to your PR, but has to do with the CI. I've restarted it, should hopefully work now.

I see there is still issues of the type I mentioned above, e.g. new_index : %(klass)s has replaced %(klass)s and many others.

let me look into it

ShisuiUzumaki · 2022-12-15T04:40:22Z

that is unrelated to your PR, but has to do with the CI. I've restarted it, should hopefully work now.

I see there is still issues of the type I mentioned above, e.g. new_index : %(klass)s has replaced %(klass)s and many others.

Is it fine now?

MarcoGorelli · 2022-12-15T08:07:43Z

Is it fine now?

Can you have a look at https://github.com/pandas-dev/pandas/pull/50225/files please? There's many unrelated changes which shouldn't be there

topper-123 · 2022-12-15T13:14:28Z

Like @MarcoGorelli says, there are still many unrelated changes. Those need to be removed.

Beside looking through https://github.com/pandas-dev/pandas/pull/50225/files, you can locally also use git to see the difference between this and the main branch. Are you using git on the terminal or in a GUI?

MarcoGorelli · 2022-12-15T13:17:38Z

I think the simplest thing at this point might be:

git fetch upstream
git reset upstream/main

Then do

git add -p

and type y for any change you want to keep, and n for any change you want to discard

Finally

git checkout -- .  # unstage the changes you discarded
git commit -m '<your commit message goes here>'
git push origin HEAD -f

ShisuiUzumaki · 2022-12-15T13:18:40Z

Like @MarcoGorelli says, there are still many unrelated changes. Those need to be removed.

Beside looking through https://github.com/pandas-dev/pandas/pull/50225/files, you can locally also use git to see the difference between this and the main branch. Are you using git on the terminal or in a GUI?

I am using pycharm. Most of these changes happened because I ran git rebase main, I should not have done that. What do you suggest that I should do?

ShisuiUzumaki · 2022-12-15T13:43:39Z

I think the simplest thing at this point might be:
git fetch upstream
git reset upstream/main
Then do
git add -p 
and type y for any change you want to keep, and n for any change you want to discard

Finally
git checkout -- .  # unstage the changes you discarded
git commit -m '<your commit message goes here>'
git push origin HEAD -f

Ok I will do that. I apologize for the inconvenience. If only I had been more careful and didn't perform 'rebase', I could have avoided this mess. I will be careful in the future. Hope I didn't bother you too much.

topper-123 · 2022-12-15T14:51:36Z

All right, looks very close now. You’ve deleted a text in the whatsnew, that should ger back in, but else, it getting very close.

ShisuiUzumaki · 2022-12-15T15:14:52Z

All right, looks very close now. You’ve deleted a text in the whatsnew, that should ger back in, but else, it getting very close.

Are you referring to this text:
- Deprecated argument ``infer_datetime_format`` in :func:to_datetime and :func:read_csv, as a strict version of it is now the default (:issue:48621)

MarcoGorelli · 2023-01-16T08:50:11Z

in case it's useful, here are the commands I would run:

git fetch upstream
git rebase upstream/main

then, fixup any conflicts

git add -u  # stage any files you fixed conflicts in
git rebase --continue

and once you get rebased successful or something like that, you can do git push origin HEAD -f

ShisuiUzumaki · 2023-01-16T09:06:15Z

It's not fully rebased.

You need to first move to the main branch and update the main branch so it corresponds to upstream/main, then move to this branch and do a git rebase main.

Have a look!

MarcoGorelli

Nice, looks good to me - over to @topper-123

MarcoGorelli

Looks like you did the rebase correctly, but there is a pre-commit check that still needs fixing up:

 Unwanted patterns in tests..............................................................................Failed
- hook id: unwanted-patterns-in-tests
- duration: 0.93s
- exit code: 1

pandas/tests/indexes/common.py:831:            "Use pandas.api.types.is_categorical_dtype instead",

topper-123 · 2023-01-16T10:23:44Z

That looks like an overzealous pre-commit check (it should not check inside strings), but I don't think regex can (elegantly) differentiate between code and string content?

IMO just delete the line "Use pandas.api.types.is_categorical_dtype instead", in pandas/tests/indexes/common.py. @MarcoGorelli?

MarcoGorelli · 2023-01-16T10:45:18Z

Looks like this check was added in #39293

IIRC match= takes a regex, so perhaps just writing "Use pandas\.api\.types\.is_categorical_dtype instead" is enough to "fool" the check?

topper-123 · 2023-01-16T11:05:54Z

Can you try that, @ShisuiUzumaki?

ShisuiUzumaki · 2023-01-16T11:59:33Z

Can you try that, @ShisuiUzumaki?

I will do that

ShisuiUzumaki · 2023-01-16T12:11:10Z

Can you try that, @ShisuiUzumaki?

Done!

MarcoGorelli

Should be good now, thanks @ShisuiUzumaki

topper-123 · 2023-01-16T13:52:35Z

Yeah, looks good. @MarcoGorelli , do you have access to rerun the Circleci arm CI?

MarcoGorelli · 2023-01-16T14:26:49Z

yeah (and I think you should too?) but if I go to https://app.circleci.com/pipelines/github/pandas-dev/pandas I don't see this branch, so can't see how to restart it

it just says

>   ???
E   ImportError: pyarrow requires pandas 0.23.0 or above, pandas 0+untagged.31208.g709e004 is installed

pyarrow/pandas-shim.pxi:65: ImportError

context deadline exceeded

though, which seems unrelated, so perhaps it's OK to just merge

topper-123 · 2023-01-16T14:57:59Z

Yeah, that must be unrelated. I’ ok with merging this, as it’s quite simple.

topper-123 · 2023-01-16T15:01:22Z

Thanks, @ShisuiUzumaki.

If this has given you a taste for more, you’re welcome to take on other issues:-)

ShisuiUzumaki added 2 commits December 14, 2022 11:10

assertion error fix added in pandas/core/indexes/base.py

3a6cfca

Merge remote-tracking branch 'origin/main'

d69d13b

Merge branch 'main' of https://github.com/ShisuiUzumaki/pandas

9ce4f09

Merge branch 'main' of https://github.com/ShisuiUzumaki/pandas

d5f0fbb

MarcoGorelli mentioned this pull request Dec 15, 2022

BUG: out-of-bounds pydatetime with errors='coerce' and non-ISO format coerces all elements to NaT #50255

Closed

3 tasks

ShisuiUzumaki force-pushed the inde_api_deprecated_is_categorical branch from 0865ed0 to e1a2973 Compare December 15, 2022 14:43

ShisuiUzumaki closed this Dec 15, 2022

ShisuiUzumaki reopened this Dec 15, 2022

Merge branch 'main' of https://github.com/ShisuiUzumaki/pandas

491616b

ShisuiUzumaki force-pushed the inde_api_deprecated_is_categorical branch from f984113 to 78e25cf Compare January 16, 2023 09:05

MarcoGorelli approved these changes Jan 16, 2023

View reviewed changes

MarcoGorelli requested changes Jan 16, 2023

View reviewed changes

ShisuiUzumaki and others added 7 commits January 16, 2023 17:07

Merge branch 'main' of https://github.com/ShisuiUzumaki/pandas

c27d97c

unrelated changes removed

e79aaca

removing unrelated changes

34af5a6

replaced self with self.dtyp in base.py, removed extra line in 2.0.0.rst

633c48f

added new line which was removed accidentally

947284c

matching warning messege in test

bc36d1f

fix for pre-commit check

709e004

ShisuiUzumaki force-pushed the inde_api_deprecated_is_categorical branch from 78e25cf to 709e004 Compare January 16, 2023 12:10

MarcoGorelli approved these changes Jan 16, 2023

View reviewed changes

MarcoGorelli added this to the 2.0 milestone Jan 16, 2023

MarcoGorelli added the Deprecate Functionality to remove in pandas label Jan 16, 2023

topper-123 added the Index Related to the Index class or subclasses label Jan 16, 2023

topper-123 approved these changes Jan 16, 2023

View reviewed changes

topper-123 merged commit 316c8ba into pandas-dev:main Jan 16, 2023

topper-123 mentioned this pull request Apr 2, 2023

DEPR: List of deprecations to be removed in 3.0 #50578

Open

DEPR: deprecate Index.is_categorical #50225

DEPR: deprecate Index.is_categorical #50225

Conversation

ShisuiUzumaki commented Dec 13, 2022 • edited Loading

ShisuiUzumaki commented Dec 13, 2022

topper-123 commented Dec 13, 2022

topper-123 commented Dec 13, 2022

ShisuiUzumaki commented Dec 14, 2022

topper-123 commented Dec 14, 2022

topper-123 commented Dec 14, 2022

ShisuiUzumaki commented Dec 14, 2022

topper-123 commented Dec 14, 2022

ShisuiUzumaki commented Dec 14, 2022 • edited Loading

topper-123 commented Dec 14, 2022

ShisuiUzumaki commented Dec 14, 2022

ShisuiUzumaki commented Dec 14, 2022

topper-123 commented Dec 14, 2022

ShisuiUzumaki commented Dec 14, 2022

ShisuiUzumaki commented Dec 15, 2022

MarcoGorelli commented Dec 15, 2022

topper-123 commented Dec 15, 2022

MarcoGorelli commented Dec 15, 2022

ShisuiUzumaki commented Dec 15, 2022

ShisuiUzumaki commented Dec 15, 2022

topper-123 commented Dec 15, 2022

ShisuiUzumaki commented Dec 15, 2022

MarcoGorelli commented Jan 16, 2023

ShisuiUzumaki commented Jan 16, 2023

MarcoGorelli left a comment

Choose a reason for hiding this comment

MarcoGorelli left a comment

Choose a reason for hiding this comment

topper-123 commented Jan 16, 2023

MarcoGorelli commented Jan 16, 2023

topper-123 commented Jan 16, 2023

ShisuiUzumaki commented Jan 16, 2023

ShisuiUzumaki commented Jan 16, 2023

MarcoGorelli left a comment

Choose a reason for hiding this comment

topper-123 commented Jan 16, 2023 • edited Loading

MarcoGorelli commented Jan 16, 2023

topper-123 commented Jan 16, 2023

topper-123 commented Jan 16, 2023

ShisuiUzumaki commented Dec 13, 2022 •

edited

Loading

ShisuiUzumaki commented Dec 14, 2022 •

edited

Loading

topper-123 commented Jan 16, 2023 •

edited

Loading