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

DEPR: deprecate Index.is_interval #50196

Merged
merged 15 commits into from Jan 24, 2023

Conversation

ShisuiUzumaki
Copy link
Contributor

@ShisuiUzumaki
Copy link
Contributor Author

@topper-123 could you review

@topper-123
Copy link
Contributor

Looks good. The failure looks unrelated, but the ARM tests should be run again just in case.

Like @phofl said in #50178, it may be a while before we merge. I can keep an eye on this getting in in due time, if you want to focus on other things.

@topper-123 topper-123 added Deprecate Functionality to remove in pandas Index Related to the Index class or subclasses labels Dec 12, 2022
@topper-123 topper-123 added this to the 2.0 milestone Dec 12, 2022
Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @ShisuiUzumaki

@phofl I see there are just few deprecations in #30228 pending enforcement. Are we still waiting for them to merge PRs with new deprecations?

@ShisuiUzumaki You'll have to merge main and fix the conflicts in what's new before we can merge.

@phofl
Copy link
Member

phofl commented Jan 4, 2023

cc @jbrockmendel I think we talked about this as well. Haven't followed the deprecations that closely, do you think we can merge new ones?

@jbrockmendel
Copy link
Member

Ideally I'd like to get #49560 and #49715 in before adding new deprecations, but not a huge deal. Other than that I'm fine with it as long as @mroeschke is.

@mroeschke
Copy link
Member

Yeah ideally I would like to get all the old 1.x deprecations out before adding new ones, but that might not be realistic to block given wanting to release 2.0 next month or so.

I would be fine with merging new ones soon as long as we start a new deprecation tracker for 3.0 issue documenting these new ones

@ShisuiUzumaki
Copy link
Contributor Author

ShisuiUzumaki commented Jan 5, 2023

lgtm, thanks @ShisuiUzumaki

@phofl I see there are just few deprecations in #30228 pending enforcement. Are we still waiting for them to merge PRs with new deprecations?

@ShisuiUzumaki You'll have to merge main and fix the conflicts in what's new before we can merge.

Which branch do I have to merge main into? Are you referring to ShisuiUzumaki:index_dtype_api_deprecation? But the merge seems to create a lot of errors, I mean that many checks are falling on the GitHub side

@datapythonista
Copy link
Member

I would be fine with merging new ones soon as long as we start a new deprecation tracker for 3.0 issue documenting these new ones

I created #50578 to track new deprecations. I also created #50579 to keep track of deprecations pending enforcement. What it makes sense to me is to have a detailed look at the code base, see what else is pending enforcement, update #50579, and when we're confident we've got everything pending enforcement under control, then we can start merging PRs with new deprecations. @mroeschke @phofl @jbrockmendel is this more or less what you have in mind to hold merging new deprecations before the rest are enforced? To make it easier to verify we don't forget any?

@phofl
Copy link
Member

phofl commented Jan 12, 2023

Needs a rebase, good to merge afterwards

@topper-123
Copy link
Contributor

Ping. Can you rebase.

@ShisuiUzumaki
Copy link
Contributor Author

Ping. Can you rebase.

On it!

@ShisuiUzumaki
Copy link
Contributor Author

ShisuiUzumaki commented Jan 23, 2023

Ping. Can you rebase.

Could you have a look? Also, I should mention that there were some conflicts in the .pre-commit-config.yaml file.

@topper-123
Copy link
Contributor

YEah, you need to fix the ‘. pre-commit-config.yaml ‘, else looks good and ready to merge.

@ShisuiUzumaki
Copy link
Contributor Author

YEah, you need to fix the ‘. pre-commit-config.yaml ‘, else looks good and ready to merge.

I think it is done

@topper-123
Copy link
Contributor

There is still a change in '.pre-commit-config.yaml' in this PR?

@ShisuiUzumaki
Copy link
Contributor Author

There is still a change in '.pre-commit-config.yaml' in this PR?

You mean from the main branch?

@topper-123
Copy link
Contributor

If you go to https://github.com/pandas-dev/pandas/pull/50196/files, you can see there's still a change in the file .pre-commit-config.yaml. Can you remove that change.

@ShisuiUzumaki
Copy link
Contributor Author

If you go to https://github.com/pandas-dev/pandas/pull/50196/files, you can see there's still a change in the file .pre-commit-config.yaml. Can you remove that change.

Sure!

@ShisuiUzumaki
Copy link
Contributor Author

If you go to https://github.com/pandas-dev/pandas/pull/50196/files, you can see there's still a change in the file .pre-commit-config.yaml. Can you remove that change.

Is it good to go now?

@topper-123 topper-123 merged commit be260f1 into pandas-dev:main Jan 24, 2023
@topper-123
Copy link
Contributor

Yeah, it's good, Thanks @ShisuiUzumaki.

pooja-subramaniam pushed a commit to pooja-subramaniam/pandas that referenced this pull request Jan 25, 2023
* assertion error fix added in pandas/core/indexes/base.py

* changes made to pandas/core/infexes/base.py for is_interval deprecation in class Index

* changes made to pandas/tests//indexes/common.py to test deprecation of is_interval

* docs/source/whatsnew/v2.0.0.rst edited for index.is_interval

* changes in pandas/core/indexes/base.py - removed extra import of is_interva;_dtype

* brnahc state before rebase

* Fixed error in .pre-commit-config.yaml file

* unwanted changed fixed in .pre-commit-cofig.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas Index Related to the Index class or subclasses
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants