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

[ENH] Address deprecated references in datatypes module #2082

Closed
wants to merge 9 commits into from

Conversation

gchhablani
Copy link

@gchhablani gchhablani commented Feb 18, 2022

Reference Issues/PRs

Fixes #2049.

What does this implement/fix? Explain your changes.

In this fix, I:

  • Removed all references of pd.Int64Index across all files.
  • Updated the tests so verify against pd.Index and dtype=='int64'.
  • Fixed all instantiations from pd.Int64Index(...) to pd.Index(..., dtype='int64').

Does your contribution introduce a new dependency? If yes, which one?

NA

What should a reviewer concentrate their feedback on?

  • Whether the fix looks okay.
  • If there are any missing entries that should be changed.
  • If there are other places where the change affects the code-base.

Any other comments?

  • There were two places where docstrings were missing because of which pre-commit was failing. I have added the docstrings as well. If it needs to be added in a separate PR, please let me know.

PR checklist

For all contributions
  • I've added myself to the list of contributors.
  • Optionally, I've updated sktime's CODEOWNERS to receive notifications about future changes to these files.
  • I've added unit tests and made sure they pass locally

@fkiraly
Copy link
Collaborator

fkiraly commented Feb 18, 2022

@gchhablani, I took the liberty of fixing a linting error to get the tests to run, hope that's ok

@gchhablani
Copy link
Author

gchhablani commented Feb 18, 2022

@fkiraly Thank a lot! I missed that 😥

@khrapovs
Copy link
Contributor

This PR looks quite promising actually. It covers much more that what I did here: #2339. It should definitely be revitalized.

@gchhablani
Copy link
Author

@khrapovs @fkiraly I can get back onto it if that is still a possibility?
Wdyt?

@khrapovs
Copy link
Contributor

@gchhablani What do you mean "a possibility"? The issue is still open. Warnings are all over the place. Your PR seem to be resolving a lot of them. So why did you stop?

@Ennosigaeon
Copy link

I would also appreciate getting this PR merged. The warnings are quite "verbose"...

@khrapovs
Copy link
Contributor

I have partially addressed those warnings in #2339, but this PR seem to be more comprehensive.

@gchhablani
Copy link
Author

gchhablani commented Mar 29, 2022

@khrapovs I was distracted by other things, but I can continue working on this one and get it merged asap. Just wanted to check with you if that is okay.

@@ -138,8 +144,8 @@ def test_check_fh_values_duplicate_input_values(arg):
ForecastingHorizon(arg)


GOOD_ABSOLUTE_INPUT_ARGS = (
pd.Int64Index([1, 2, 3]),
GOOD_INPUT_ARGS = (

Choose a reason for hiding this comment

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

You also have to rename the variable in line 151

@khrapovs
Copy link
Contributor

khrapovs commented Apr 5, 2022

This PR can be closed. The issue #2332 is resolved. Sorry, @gchhablani, these warnings were too annoying.

@fkiraly
Copy link
Collaborator

fkiraly commented Apr 5, 2022

yes, I think they are, and this is now heavily conflicting.

Thanks a lot for your work, @gchhablani.
Feel free to add yourself as a code contributor since @khrapovs' work was partially based on yours.

@fkiraly fkiraly closed this Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENH] Address deprecated references in datatypes module
4 participants