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

GH1055 Add pandas.api.typing to pandas-stubs #1058

Merged
merged 9 commits into from
Dec 1, 2024

Conversation

loicdiridollou
Copy link
Contributor

@loicdiridollou
Copy link
Contributor Author

Haven't written tests for every case, I first want to get feedback on the structure and if I am going the right direction.

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

If we can't do assert_type on the pandas.api.typing objects, then we may have to change the stubs to support that.

tests/test_api_typing.py Outdated Show resolved Hide resolved
tests/test_api_typing.py Outdated Show resolved Hide resolved
tests/test_api_typing.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

So this structure is fine. Now you need to add tests for all of the other things in pandas.api.typing

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Nov 28, 2024

So I see that your tests failed and I played around with it. The purpose of the types in pandas.api.typing is to allow people to use the types in functions. Because we are doing things with Generic, we have to change the test. E.g., for DataFrameGroupBy:

def test_dataframegroupby():
    df = pd.DataFrame({"a": [1, 2, 3]})
    group = df.groupby("a")

    def f1(gb: DataFrameGroupBy):
        check(group, DataFrameGroupBy)

    f1(group)

We can't use the check(assert_type pattern here because of the Generic. But for this particular part of the API, it would be good to add a test like the one above, because that is the anticipated usage. This will do a typing check via the function call and an isinstance check as a result of the use of check .

You'd only have to do this for the ones that are Generic inside of the stubs. Otherwise the standard check(assert_type(... pattern is fine, like you did with NAType

@loicdiridollou
Copy link
Contributor Author

Thanks for the trick here!
One issue is that one of the type can only be obtained through a deprecated resampling, not sure what to do about this one.

tests/test_api_typing.py Outdated Show resolved Hide resolved
tests/test_api_typing.py Outdated Show resolved Hide resolved
tests/test_api_typing.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

one small change to use TypeAlias

tests/test_api_typing.py Outdated Show resolved Hide resolved
@loicdiridollou loicdiridollou requested a review from Dr-Irv December 1, 2024 00:36
Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

@Dr-Irv Dr-Irv merged commit d4d399e into pandas-dev:main Dec 1, 2024
10 checks passed
@loicdiridollou loicdiridollou deleted the gh1055_api_typing branch December 14, 2024 19:58
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.

Add pandas.api.typing module
2 participants