Skip to content

Refactor inherit_names to work by inheritance #32100

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

Open
SaturnFromTitan opened this issue Feb 19, 2020 · 2 comments
Open

Refactor inherit_names to work by inheritance #32100

SaturnFromTitan opened this issue Feb 19, 2020 · 2 comments
Labels
Typing type annotations, mypy/pyright type checking

Comments

@SaturnFromTitan
Copy link
Contributor

We face a lot of mypy errors due to the dynamic nature of inherit_names. See issue #31716 and the discussion that evolved there.

Summary:
In the past, we added explicit type annotations to the respective classes, but this approach doesn't scale.

As mypy is a static type checker, it cannot understand the dynamic nature of inherit_names by design. The only alternative would be to refactor inherit_names to work by inheritance instead of dynamic assignments. This would make the code more understandable to humans as well.

@simonjayhawkins simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Feb 19, 2020
@simonjayhawkins simonjayhawkins added this to the Contributions Welcome milestone Sep 30, 2020
@twoertwein
Copy link
Member

Convincing pyright/mypy to infer "inherited" methods sounds unlikely microsoft/pyright#3812.

Using mixin sub-classes to out-source common methods/properties definitely sounds like the best way to go! Is that still the ideal way to deal with that? @simonjayhawkins

I can imagine that those mixin-classes might need to be abstract as the shared methods might refer to methods/properties not existing in those otherwise empty mixin-classes (or we add many mypy ignore comments inside those methods).

There are luckily only a "few" classes making use of inherit_names: IntervalIndex, DatetimeIndexOpsMixin, CategoricalIndex, PeriodIndex, TimedeltaIndex, DatetimeIndex.

@twoertwein
Copy link
Member

Using mixin sub-classes to out-source common methods/properties definitely sounds like the best way to go!

seems that isn't possible as self needs to be self._data

@mroeschke mroeschke removed this from the Contributions Welcome milestone Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

No branches or pull requests

4 participants