Skip to content

TYP: PoC for honest ABC #37540

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

Closed
wants to merge 3 commits into from

Conversation

rhshadrach
Copy link
Member

PoC implementation for using honest ABCs for pandas objects. Currently only implements ABCNDFrame. Compare:

def foo(obj: ABCNDFrame):
    reveal_type(obj.attrs)


def bar(obj: ABCDataFrame):
    reveal_type(obj.attrs)

In this PR, mypy will output Revealed type is 'builtins.dict[Union[typing.Hashable, None], Any]' for the former whereas the latter is Revealed type is 'Any'.

A script can be written (and I plan to, if this idea gets buy-in) to generate the ABC class off of the concrete class. Likewise, tests can be written to ensure that ABCNDFrame and NDFrame don't diverge.

docstrings can be removed from the ABC class; I just wanted to highlight that we could just have the docs in the ABC class (don't know if that would be desirable).

cc @simonjayhawkins, @jreback, @jbrockmendel

@jbrockmendel
Copy link
Member

This is a big change. Why not just put in dtypes.generic

class ABCNDFrame:
    pass

and then "honestly" have NDFrame subclass that?

@rhshadrach
Copy link
Member Author

The idea here is to allow mypy to be aware of method signatures when doing e.g. isinstance(obj, ABCNDFrame). I believe this is only possible by copying the signatures of the methods of NDFrame into ABCNDFrame.

The diff will go down dramatically if docstrings are removed.

@rhshadrach
Copy link
Member Author

I removed docstrings to make the diff easier to understand.

@simonjayhawkins
Copy link
Member

related issue #27353

A script can be written (and I plan to, if this idea gets buy-in) to generate the ABC class off of the concrete class. Likewise, tests can be written to ensure that ABCNDFrame and NDFrame don't diverge.

stubgen can generate stub files automatically.

I think we should maybe investigate creating stub files in the first instance, see #27353 (comment)

These may need to be manually curated in the first instance. But I think using stubgen could reduce the need for custom scripts and checks longer term.

@rhshadrach
Copy link
Member Author

rhshadrach commented Nov 6, 2020

@simonjayhawkins Thanks for the comments and links. Using cast on the ABCs did not occur to me, even if we decide not to go with stubs it seems clear to me that cast is a better solution than what is being done here.

@rhshadrach rhshadrach closed this Nov 6, 2020
@rhshadrach rhshadrach deleted the honest_abcndframe branch November 8, 2020 15:11
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.

3 participants