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

Improved the type stubs in the _libs directory to help with type checking #43744

Closed
wants to merge 2 commits into from

Conversation

erictraut
Copy link

This doesn't change any code, so it should have no impact on runtime behavior.

@@ -0,0 +1,3 @@

cache_readonly = property
Copy link
Member

@twoertwein twoertwein Sep 25, 2021

Choose a reason for hiding this comment

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

I think this will need many ignore statements, since there are quite a few places where a typed cache_readonly will reveal existing type issues (until now, mypy couldn't find cache_readonly).

I think you will also need to add annotations for AxisProperty as that is important elsewhere. It would be great to have partial type-stubs.

@twoertwein twoertwein added the Typing type annotations, mypy/pyright type checking label Sep 25, 2021
VALID_CLOSED: FrozenSet[str]

def intervals_to_interval_bounds(
intervals: np.ndarray, valdiate_closed: bool = ...
Copy link
Member

Choose a reason for hiding this comment

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

valdiate_closed -> validate_closed

@property
def rule_code(self) -> str: ...
def freqstr(self) -> str: ...
# Next one is problematic due to circular imports
Copy link
Member

Choose a reason for hiding this comment

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

do it inside a if TYPE_CHECKING block

@@ -0,0 +1,8 @@
class C_NAType: ...
Copy link
Member

Choose a reason for hiding this comment

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

is there any benefit/reason to include the C class.

@@ -13,6 +13,7 @@ iNaT: int
nat_strings: set[str]

def is_null_datetimelike(val: object, inat_is_null: bool = ...) -> bool: ...
def checknull_with_nat(val: object) -> bool: ...
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to expose this to python code.

@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Oct 27, 2021
@twoertwein
Copy link
Member

@erictraut If you don't have time for this PR, I wouldn't mind forking your branch, rebasing it, and making adjustments to get it merged (after #43828 is merged).

Out of curiosity: Did pyright generate the type annotations in this PR or are they from Microsoft's pandas type stubs?

@erictraut
Copy link
Author

@twoertwein, thanks for offering. Yeah, I'd appreciate it if you'd take over this PR. Addressing its issues involved more complexity than I anticipated.

These type annotations are based on Microsoft's pandas type stubs plus some additional hand-written signatures. They weren't auto-generated by pyright.

@jreback
Copy link
Contributor

jreback commented Dec 16, 2021

pls rebase

@twoertwein
Copy link
Member

pls rebase

I think this PR could be closed (but please don't delete the fork). I cherry-pick parts of this PR into more manageable portions.

@jreback
Copy link
Contributor

jreback commented Jan 16, 2022

closing as per @twoertwein comment #43744 (comment)

@jreback jreback closed this Jan 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants