ATM we vendor typing_extensions but mypy doesnt recognize it, so we can't actually use it. We should either remove it or make it a dependency.
Comment From: TomAugspurger
So if I do mypy pandas on a file using something from our vendored typing extensions, mypy will error? Or ignore the types?
Comment From: jorisvandenbossche
Can you also explain what it means to "make it a dependency"? Is that only a developer dependency for when you want to do type checking? Or would it actually be a runtime dependency?
Comment From: jbrockmendel
So if I do mypy pandas on a file using something from our vendored typing extensions, mypy will error? Or ignore the types?
If I import from pandas._vendored.typing_extensions import final and then incorrectly decorate Index.get_loc with @final, mypy will fail to alert me that this is incorrect.
Comment From: jbrockmendel
Can you also explain what it means to "make it a dependency"? Is that only a developer dependency for when you want to do type checking? Or would it actually be a runtime dependency?
I'm not aware of any way to make it a dev-only dependency. AFAIK it would have to be runtime-importable.
Comment From: jorisvandenbossche
And the import can't be hidden behind a if TYPE_CHECKING clause as we do for other cases, because it are actual functions used as decorators and such, I suppose?
We could have a no-op version of the items we want to use, which we import at runtime, and override them with the actual working versions from typing_extensions behind if TYPE_CHECKING ?
Comment From: jbrockmendel
And the import can't be hidden behind a if TYPE_CHECKING
That's a neat idea, but doesn't do it.
Comment From: jorisvandenbossche
And can you explain why it doesn't do it? (see also my second sentence in the comment above)
(I don't know much about typing, just asking questions)
Comment From: simonjayhawkins
And can you explain why it doesn't do it? (see also my second sentence in the comment above)
i did this in https://github.com/pandas-dev/pandas/pull/27646/files/9aa69fe5491e0b161e679c8f54cbb8f13a48b85a
didn't get any support at the time follow conversation from https://github.com/pandas-dev/pandas/pull/27646#issuecomment-518290328 onwards
Comment From: simonjayhawkins
see also https://github.com/pandas-dev/pandas/issues/34869#issuecomment-669075742 for more discussion and a reference to similar workaround used by pytest for Protocol.
Comment From: jorisvandenbossche
So that seems certainly doable then to have mypy_extensions as a dev dependency, without having it as a runtime dependency
Comment From: simonjayhawkins
and we no longer need to add it explicitly to environment.yaml see https://github.com/pandas-dev/pandas/pull/27646#issuecomment-530970654
Comment From: jbrockmendel
And can you explain why it doesn't do it?
I don't know, can only confirm that I tried it and it didn't work.
see also #34869 (comment) for more discussion and a reference to similar workaround used by pytest for Protocol.
@simonjayhawkins this looks really similar to Joris's suggestion which I was not able to implement succesfully. My prior has nonzero weight on "me messing up", can you confirm whether or not this is viable for us?
Comment From: simonjayhawkins
I'm not sure if we could do everything that we could do if having typing_extensions as a runtime dependency would allow. but if having a runtime dependency is a definite no-no, then some more investigation of possible workarounds is worthwhile.
as a start, have opened #37137 as a POC for Literal in function signatures.
Comment From: MarcoGorelli
if having a runtime dependency is a definite no-no
Is it definitely a no-no? Not that this is a definite reason to include it, but for reference, black use it:
https://github.com/psf/black/blob/df4dd38a9a2b44c14485948fe8209fa19db2383a/setup.py
install_requires=[
"click>=7.1.2",
"appdirs",
"toml>=0.10.1",
"typed-ast>=1.4.2",
"regex>=2020.1.8",
"pathspec>=0.6, <1",
"dataclasses>=0.6; python_version < '3.7'",
"typing_extensions>=3.7.4; python_version < '3.8'",
"mypy_extensions>=0.4.3",
],
Comment From: simonjayhawkins
re-opening as discussion about having typing_extensions as a dev only dependency or using other workarounds is ongoing.
Comment From: MarcoGorelli
Hi @simonjayhawkins - why not make it a runtime dependency? Wouldn't that be needed to be able to access Literal and Protocol in Python3.7?
I realise that there needs to be a really high bar for adding an extra runtime dependency, and this is one case where I think it would really be worth it:
- it means being able to overload functions with inplace: bool so that we know what the return type is
- tighter typing potentially uncovers bugs
- typing-extensions is really light - we're talking about a handful of kilobytes https://pypi.org/project/typing-extensions/#files . It has no requirements itself and so won't conflict with anything
- PyTorch and black have it as a non-optional dependency. I'm not advocating for cargo-culting, just pointing out that there's already a precedent for including it as a requirement
Perhaps this is best discussed in the next dev meeting? tagging @pandas-dev/pandas-core @pandas-dev/pandas-triage in case anyone hasn't seen this and has input
EDIT: just saw #37137, which Literal is indeed used successfully, so my first point no longer stands
Comment From: bashtage
You can just dummy these for Python < 3.8
Something like
from __future__ import annotations
import sys
from typing import TYPE_CHECKING, List
if sys.version_info >= (3,8):
from typing import Literal
if TYPE_CHECKING:
from typing_extensions import Literal
Edit: Soft dependency only needed if developing/type checking on Python 3.7
Comment From: MarcoGorelli
@bashtage from #37137 it seems that this is even simpler if you have from __future__ import annotations, so...perhaps this can be closed for now? Can reopen if some limitation of this approach comes up
Comment From: bashtage
@MarcoGorelli I don't think annotations fixes the missing Literal, at least if you want to be able to type check in Python 3.7.
Comment From: MarcoGorelli
See #37137 though, it seems to work fine there
e.g.
from __future__ import annotations
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from typing import Literal
def foo(a: Literal[True]) -> None: ...
runs fine in Python3.7, but fails if you remove from __future__ import annotations
Comment From: simonjayhawkins
Hi @simonjayhawkins - why not make it a runtime dependency? Wouldn't that be needed to be able to access
LiteralandProtocolin Python3.7?
This has been discussed many times before. I have no objection, but others do.
by all means we can discuss again, but would need to get @TomAugspurger and @WillAyd to agree
Comment From: bashtage
Does it type check in Python 3.7?
Comment From: TomAugspurger
Since we're a central part of the scientific python ecosystem, we should have a high bar for taking on dependencies. IMO this doesn't meet that bar.
Comment From: MarcoGorelli
Does it type check in Python 3.7?
As in, can you run mypy? typing_extensions is a required dependency of mypy, so yes, it should be fine.
Anyway, it seems that
if TYPE_CHECKING:
from typing import Literal
works for both running the code, and for type checking it, so it should be OK to avoid adding this as a requirement.
If all that needs to be done until Python3.7 is dropped is to place some imports under if TYPE_CHECKING, then we don't need this.
I think this can be closed then - if placing these imports under TYPE_CHECKING has some limitation, then we can reopen and reevaluate whether this meets the bar for inclusion
Comment From: topper-123
My experince in using import typing_extensions inside if TYPE_CHECKING in #51480 and #51309 is that everything works as expected, i.e. mypy used the relevant typing_extensionsclasses for typing and did catch a lot of things when used.
Maybe someone could corroborate my experiece and see if they see something I didn't?
Comment From: topper-123
typing_extensions as a dev dependency has been merged into main in #51480, so closing this issue.