TYP ensure bool_t is always used in pandas/core/generic.py

[MarcoGorelli]

• Ensure all linting tests pass, see here for how to run them

Currently, lots of methods use the bool annotation in that file. I'm not sure it affects mypy (some experiments with reveal_type suggest it doesn't) - however, it does confuse VSCode, which takes you to the method bool if you hover over one of these annotations.

The file already contains

bool_t = bool  # Need alias because NDFrame has def bool:

(added in #26024), but it's not always used

[jbrockmendel]

this looks fine, but im a bit wary of the amount of code being added

[MarcoGorelli]

Thanks @jbrockmendel - I share your concern, I just didn't find this to be feasible with a regular expression, and as pandas is using static typing more and more so I figured it'd be worthwhile to enforce using the correct type in that file.
No objection to closing if you think it's not worth it though

[simonjayhawkins]

The file already contains

bool_t = bool  # Need alias because NDFrame has def bool:

(added in #26024), but it's not always used

IIRC the alias is only needed after the bool method is defined and within the scope of the class definition.

however, it does confuse VSCode

possibly a VSCode bug? are you using pylance? I'm getting taken to the typing definitions at times instead of the code definition, but it's still in preview for now.

[MarcoGorelli]

shall we just remove bool_t = bool and use bool consistently then? (I think this'd be my preferred solution, am just seeing where it's really needed)

[simonjayhawkins]

see #32365 for a bit of history.

maybe could move bool method to end of class definition to reduce bool_t usgaes.

[MarcoGorelli]

IIRC the alias is only needed after the bool method is defined and within the scope of the class definition.

If I've understood correctly, then I'm only seeing this for class attributes, not for variable annotations.

The bool method is defined on line 1486.
If on line 2491 I put reveal_type(lines) (lines was annotated as bool_t), then we get

$ mypy pandas
pandas/core/generic.py:2491: note: Revealed type is 'builtins.bool'

Where we run into problems is if we put bar: bool after def: bool. Then we get

$ mypy pandas/core/generic.py 
pandas/core/generic.py:1530: error: Function "pandas.core.generic.NDFrame.bool" is not valid as a type  [valid-type]
pandas/core/generic.py:1530: note: Perhaps you need "Callable[...]" or a callback protocol?
Found 1 error in 1 file (checked 1 source file)

However, pandas typically defines class attributes at the top of the class, before the various methods, so this wouldn't be an issue. And if it was, mypy would complain loudly (as above), it wouldn't let us assign a method as an annotation.

So removing bool_t = bool and just using bool seems fine to me

[MarcoGorelli]

Sorry, I've done this "test" the wrong way around - if I try putting bool instead of bool_t in the annotations, then we get

pandas/core/generic.py:9629: note: Perhaps you need "Callable[...]" or a callback protocol?
pandas/core/generic.py:10916: error: Function "pandas.core.generic.NDFrame.bool" is not valid as a type  [valid-type]

[MarcoGorelli]

maybe could move bool method to end of class definition to reduce bool_t usgaes.

This works and is IMO the best solution

$ mypy pandas
Success: no issues found in 1244 source files

[simonjayhawkins]

Thanks @MarcoGorelli

[WillAyd]

Is this a documented solution? I would be slightly against doing this unless it is promoted as a fix by mypy

[MarcoGorelli]

Would you prefer to use bool_t everywhere in this file?

I don't mind, it's having both bool and bool_t in the same file that I find confusing

[WillAyd]

Yea I think using an alias is a better solution. Some background info here:

#26029 (comment)

These comments are a bit dated so maybe things have changed in the interim, but if not an alias is the suggested approach

[jbrockmendel]

Does from __future__ import annotations affect this at all?

[MarcoGorelli]

I'll check, but I don't think so

[MarcoGorelli]

Does from __future__ import annotations affect this at all?

It makes no difference - e.g.:

# t.py
from __future__ import annotations

class Cat:
    def bool(self): ...

    def other_method(self, inplace: bool): ...

results in

$ mypy t.py 
t.py:6: error: Function "t.Cat.bool" is not valid as a type  [valid-type]
t.py:6: note: Perhaps you need "Callable[...]" or a callback protocol?
Found 1 error in 1 file (checked 1 source file)

regardless of whether the future annotations are imported

---

Anyway, have revisited and updated to use bool_t everywhere in this file

[WillAyd]

lgtm. I think the pre-commit check is a little overkill but no objection to keeping

[simonjayhawkins]

Thanks @MarcoGorelli