[RFC] typing around terminal

[blueyed]

No description provided.

[blueyed]

Ref: https://www.python.org/dev/peps/pep-0589/

[bluetech]

Can do it, only issue is that unless we want to bring in a runtime dependency on typing_extension, we'll need to add a compat for it, or always surround it with if TYPE_CHECKING and string annotations.

[blueyed]

I think it is rather out of scope, since it requires py38 (IIRC).
Better options?
Otherwise I will remove the XXX/TODO comments about it.

We could also have stats_warnings explicitly for this, but that's not B/C (but could be done via a property or thelike then).
But IIRC I've tried a property based approach (where I would define the returned type), which did not help really.

[blueyed]

@bluetech any input here, please?

[bluetech]

LGTM, left some mostly optional comments.

[bluetech]

I think that adding an annotation to a function whose own signature is unannotated doesn't have an effect (ref).

So it is better to add this annotation to the location parameter instead, if possible. This will make mypy consider the function to be typed.

[blueyed]

reveal_type gives me 'Tuple[builtins.str, Union[builtins.int, None], builtins.str]' here (mypy 0.750+dev.84126ab9985844d8cda3809688034af895fdaf7c.dirty).

This appears to come / being enabled via BaseReport - adding it there (and to the def here) instead.

[blueyed]

But it is needed to fix "List or tuple expected as variable arguments" with https://github.com/blueyed/pytest/blob/0133097943e0140ab82e07d4d201405145d35e59/src/_pytest/terminal.py#L442.
But it seems strange that with "location: Tuple[str, Optional[int], str]" in TestReport.__init__ and rep being _pytest.reports.TestReport, rep.location is Union[Tuple[builtins.str, Union[builtins.int, None], builtins.str], None] then?

The problem appears to be that BaseReport has location = None # type: Optional[Tuple[str, Optional[int], str]], and we cannot have location: Tuple[str, Optional[int], str] in TestReport (py35).
Adding an assert appears to work also, but then again it is better to keep the type comment there.

[bluetech]

-> None

[blueyed]

Is there a way to have mypy complain about it?

[bluetech]

Would be nice to have a short comment on why the type: ignore is needed here. This why we'll be able to tell later whether it's a "TODO" or a "workaround mypy" thing.

[blueyed]

It's a workaround-xdist thing (I assume that xdist just puts it there).

[blueyed]

I'll keep it as is (it is somehow obvious from "xdist" in there, and I do not like extra comments for this above). Also it might get reported when not being used/needed later anymore (there's a mode in mypy to do so).

[bluetech]

Can do it, only issue is that unless we want to bring in a runtime dependency on typing_extension, we'll need to add a compat for it, or always surround it with if TYPE_CHECKING and string annotations.