Skip to content

TYP: Narrow down type of method keyword in Index #52723

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
Merged
merged 5 commits into from
May 7, 2023
Merged

TYP: Narrow down type of method keyword in Index #52723

merged 5 commits into from
May 7, 2023

Conversation

kklein
Copy link
Contributor

@kklein kklein commented Apr 18, 2023 •
edited
Loading

This PR narrows down the type of the method keyword, used in two methods of the class Index to exactly the permitted values via the use of Literal.

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • 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/vX.X.X.rst file if fixing a bug or adding a new feature.

@kklein kklein changed the title Narrow down types of method in Index TYP: Narrow down types of method in Index Apr 18, 2023
@kklein kklein changed the title TYP: Narrow down types of method in Index TYP: Narrow down type of method keyword in Index Apr 18, 2023
@kklein
Copy link
Contributor Author

kklein commented Apr 18, 2023

@phofl @noatamir @jorisvandenbossche @MarcoGorelli PyData workshop

@noatamir noatamir added the Sprints Sprint Pull Requests label Apr 18, 2023
@kklein
Copy link
Contributor Author

kklein commented Apr 18, 2023

Any suggestions on how to proceed with clean_reindex_fill_method and clean_fill_method?

@twoertwein
Copy link
Member

Didn't see that mypy failed:

pandas/core/indexes/base.py:3749: error: Incompatible types in assignment (expression has type "Optional[str]", variable has type "Optional[Literal['pad', 'ffill', 'backfill', 'bfill', 'nearest']]") [assignment]

You might be able to change the signature of clean_reindex_fill_method or add an ignore comment.

@phofl
Copy link
Member

phofl commented Apr 21, 2023

Yep you can change the signature as well

@kklein
Copy link
Contributor Author

kklein commented May 6, 2023 •
edited
Loading

@phofl @twoertwein
Sorry about the delay!

The mypy/typing problem seems to be resolved, yet one unit tests is failing. Could it be that it's flaky? I haven't been able to deduce much from the error message.

@twoertwein twoertwein merged commit 8ef94a0 into pandas-dev:main May 7, 2023
@twoertwein
Copy link
Member

Thank you @kklein !

Rylie-W pushed a commit to Rylie-W/pandas that referenced this pull request May 19, 2023
@kklein
TYP: Narrow down type of method keyword in Index (pandas-dev#52723)
842b57d 
* Add literal type hint.

* Move type variable to _typing.py.

* Use existing type alias.

* EOF
Daquisu pushed a commit to Daquisu/pandas that referenced this pull request Jul 8, 2023
@kklein
TYP: Narrow down type of method keyword in Index (pandas-dev#52723)
563cf3f 
* Add literal type hint.

* Move type variable to _typing.py.

* Use existing type alias.

* EOF
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@twoertwein twoertwein twoertwein approved these changes

Assignees
No one assigned
Labels
Sprints Sprint Pull Requests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kklein @twoertwein @phofl @noatamir