TYP: Use Self instead of class-bound TypeVar in pyi files (remove Y019) #51480

topper-123 · 2023-02-18T19:29:47Z

Progress towards #51233. Next up will be to use Self in the pandas code base.

…9 ignore)

twoertwein · 2023-02-18T20:50:11Z

environment.yml

@@ -117,3 +117,4 @@ dependencies:

  - pip:
      - sphinx-toggleprompt
+      - typing_extensions; python_version<"3.11"


I think we don't need to add it, as it is only used during type checking (and typing_extensions is, I think, a dependency of mypy).

Yes, typing_extension is a dependency of mypy, I just checked for that. We do not want to be explicit, now that we're importing fro typing_extensions?

I think even the stubs packages from typeshed (for example https://pypi.org/project/types-psutil/) do not have typing_extensions as a dependency (neither does pandas-stubs).

If it is a new dependency, it probably should be added to the whatsnew (there might be other places where it needs to be added as well).

we have had discussions about this before and the outcome was that we do not use typing_extensions.

IIUC we don't need it with the latest mypy (1.0.0) when checking with python 3.11

twoertwein · 2023-02-18T20:53:06Z

requirements-dev.txt

@@ -86,4 +86,5 @@ feedparser
 pyyaml
 requests
 sphinx-toggleprompt
+typing_extensions; python_version<"3.11"


I don't think we need this

twoertwein

The Self changes look good to me. Whether typing_extensions should be a new dependency or not, should be judged by someone else.

topper-123 · 2023-02-19T15:30:49Z

The Self changes look good to me. Whether typing_extensions should be a new dependency or not, should be judged by someone else.

+1 from me, I'm also not sure what is normal.

I looked at typeshed, and the project itself has typing_extensions as a test requirement, se the requirements-tests.txt. OTOH, individual type stubs don't have it as a requirement AFAIKS, e.g. types-protobuf uses typing_extensions but has no dependencies...

simonjayhawkins · 2023-02-21T13:28:38Z

environment.yml

@@ -117,3 +117,4 @@ dependencies:

  - pip:
      - sphinx-toggleprompt
+      - typing_extensions; python_version<"3.11"


we have had discussions about this before and the outcome was that we do not use typing_extensions.

IIUC we don't need it with the latest mypy (1.0.0) when checking with python 3.11

simonjayhawkins · 2023-02-21T13:38:00Z

pandas/_typing.py

+    if sys.version_info >= (3, 11):
+        from typing import Self
+    else:
+        from typing_extensions import Self  # pyright: reportUnusedImport = false


I guess you could make Self equivalent to Any here. IIRC we did something similar before to enable the latest typing features

We type check using a single version of Python. There are capabilities in mypy to set the Python version to check against regardless of the version used. We don't do that.

So i'm not sure when this import is actually needed.

topper-123 · 2023-02-22T09:48:25Z

This PR was made as a bit of a follow-up to #51309, where @jbrockmendel had a suggestion to use TypeGuard for a more precise typing experience. TypeGuard is only available from python 3.10, so he added type_extensions to _typing module. #51309 hasn't actually been pulled in yet, but I didn't sense that using TypeGuard/typing_extensions as controversial.

I was not not aware of the decision to not use typing_extensions (which is totally on me, I had a break from pandas so have not been aware of some thing happening in the community). Would it be possible to reconsider the decision to not use typing_extensions?

IMO typing_extensions will be very useful to use in Pandas, because:

typings_extensions is just a backport of functionality from newer python version to allow mypy to type check using older python versions,
it is a dependency of mypy itself already (so is already installed, if mypy is installed), is developed by Guido & company (so has good support), so it doesn't seem controversial to me to use it, and
Pandas is a very large code base, so more/better typing is IMO always helpful to our project.
?

simonjayhawkins · 2023-02-22T10:47:07Z

Would it be possible to reconsider the decision to not use typing_extensions?

When we had the discussion before I was arguing for including it. Now that we have pandas-stubs and typing in the codebase could be considered as a glorified linter there are probably less arguments for. Also, it has not always ended up been a blocker for using features supported in the latest mypy by implementing some trivial workarounds in some cases.

cc @TomAugspurger @jorisvandenbossche @jreback @WillAyd from previous discussions.

simonjayhawkins · 2023-02-22T11:07:34Z

The previous decision was for a run-time dependency.

As the import is inside a TYPE_CHECKING block this implementation only makes it a dev dependency (just like the use of npt)?

topper-123 · 2023-02-22T13:57:10Z

The previous decision was for a run-time dependency.

As the import is inside a TYPE_CHECKING block this implementation only makes it a dev dependency (just like the use of npt)?

Yes, this will only be active when type checking and it will not be a run time requirement.

simonjayhawkins · 2023-02-22T14:43:34Z

scanning back over the historic typing_extensions discussions, the issue with having it as a dev-only dependency was the the need to wrap in TYPE_CHECKING and was also before typing_extensions was an required dependency of mypy in #27646

simonjayhawkins · 2023-02-24T12:25:22Z

IMO typing_extensions will be very useful to use in Pandas, because:

typings_extensions is just a backport of functionality from newer python version to allow mypy to type check using older python versions,

it is a dependency of mypy itself already (so is already installed, if mypy is installed), is developed by Guido & company (so has good support), so it doesn't seem controversial to me to use it, and

Pandas is a very large code base, so more/better typing is IMO always helpful to our project.

adding to this list...

pandas-stubs is not restricted by the minimum Python version support. It is desirable to keep the pandas-stubs and pandas type annotations in sync where possible/appropriate.

jorisvandenbossche · 2023-03-01T08:18:30Z

From looking at the linked issues, #37119 might be the most recent-ish complete discussion issue about this (+ #34869 as a precursor of it). We can maybe have the actual discussion about adding as dependency or not there?

From skimming that issue, what is still unclear to me is what the exact consequences are? Is this only a dev dependency (only contributors have to have it installed in their environment) and not a required runtime dependency? Or would it only be useful if it's actually a runtime dependency?
(the issue has various conflicting messages about this, some say it's possible, others not, or that it limits its usefulness, ..)
While from this PR it seems it is only a dev dependency, since it's not added to pyproject.toml.

Can someone summarize this over at #37119?

topper-123 · 2023-03-01T11:48:10Z

I've answered in #37119 regarding my experience using typing_extensions inside the if TYPE_CHECKING block.

Wrt. whether typing_extensions is/should be a run time dependency or not, I've made if NOT a run time dependency in this PR (i.e. only imported when TYPE_CHECKING is True).

If there's an argument for making this available at run time i'm open to arguments, but I just didn't see the need myself. and it makes sense to always limit run time dependencies when possible.

simonjayhawkins

Thanks @topper-123

doesn't appear there are any objections from anyone this time round.

if part of the argument to use typing_extensions is that it is a required dependency of mypy, do we need to explicitly include it in environment.yml

requires merge main

lgtm pending green

topper-123 · 2023-03-13T19:27:12Z

I've merged main.

WillAyd · 2023-03-13T19:29:00Z

Wow great work @topper-123

jbrockmendel · 2023-03-13T19:30:26Z

nice!

jorisvandenbossche · 2023-03-13T19:31:07Z

Can someone update #37119 with the outcome? (and probably close it)

TYP: Use Self instead of class-bound TypeVar in pyi files (remove Y01…

4307c45

…9 ignore)

twoertwein reviewed Feb 18, 2023

View reviewed changes

twoertwein approved these changes Feb 19, 2023

View reviewed changes

twoertwein requested a review from jreback February 19, 2023 16:39

This was referenced Feb 19, 2023

TYP: Use Self instead of class-bound TypeVar (generic.py/frame.py/series.py) #51493

Merged

TYP: Use Self instead of class-bound TypeVar II (pandas/core/arrays/) #51497

Merged

twoertwein added the Typing type annotations, mypy/pyright type checking label Feb 20, 2023

topper-123 mentioned this pull request Feb 21, 2023

TYP: Use Self for type checking (remaining locations) #51524

Merged

5 tasks

simonjayhawkins added this to the 2.1 milestone Feb 21, 2023

simonjayhawkins requested changes Feb 21, 2023

View reviewed changes

simonjayhawkins reviewed Feb 21, 2023

View reviewed changes

topper-123 mentioned this pull request Mar 1, 2023

typing_extensions #37119

Closed

simonjayhawkins approved these changes Mar 13, 2023

View reviewed changes

Merge branch 'master' into remove_flake8_Y019_ignore

3eef770

WillAyd merged commit b07a388 into pandas-dev:main Mar 13, 2023

topper-123 deleted the remove_flake8_Y019_ignore branch March 13, 2023 19:36

topper-123 mentioned this pull request Mar 13, 2023

TYP: misc return types #51912

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TYP: Use Self instead of class-bound TypeVar in pyi files (remove Y019) #51480

TYP: Use Self instead of class-bound TypeVar in pyi files (remove Y019) #51480

topper-123 commented Feb 18, 2023

twoertwein Feb 18, 2023

topper-123 Feb 19, 2023

twoertwein Feb 19, 2023

simonjayhawkins Feb 21, 2023

twoertwein Feb 18, 2023

twoertwein left a comment

topper-123 commented Feb 19, 2023

simonjayhawkins Feb 21, 2023

simonjayhawkins Feb 21, 2023

simonjayhawkins Feb 22, 2023

topper-123 commented Feb 22, 2023 •

edited

Loading

simonjayhawkins commented Feb 22, 2023 •

edited

Loading

simonjayhawkins commented Feb 22, 2023

topper-123 commented Feb 22, 2023

simonjayhawkins commented Feb 22, 2023

simonjayhawkins commented Feb 24, 2023 •

edited

Loading

jorisvandenbossche commented Mar 1, 2023

topper-123 commented Mar 1, 2023 •

edited

Loading

simonjayhawkins left a comment

topper-123 commented Mar 13, 2023

WillAyd commented Mar 13, 2023

jbrockmendel commented Mar 13, 2023

jorisvandenbossche commented Mar 13, 2023 •

edited

Loading

TYP: Use Self instead of class-bound TypeVar in pyi files (remove Y019) #51480

TYP: Use Self instead of class-bound TypeVar in pyi files (remove Y019) #51480

Conversation

topper-123 commented Feb 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twoertwein left a comment

Choose a reason for hiding this comment

topper-123 commented Feb 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topper-123 commented Feb 22, 2023 • edited Loading

simonjayhawkins commented Feb 22, 2023 • edited Loading

simonjayhawkins commented Feb 22, 2023

topper-123 commented Feb 22, 2023

simonjayhawkins commented Feb 22, 2023

simonjayhawkins commented Feb 24, 2023 • edited Loading

jorisvandenbossche commented Mar 1, 2023

topper-123 commented Mar 1, 2023 • edited Loading

simonjayhawkins left a comment

Choose a reason for hiding this comment

topper-123 commented Mar 13, 2023

WillAyd commented Mar 13, 2023

jbrockmendel commented Mar 13, 2023

jorisvandenbossche commented Mar 13, 2023 • edited Loading

topper-123 commented Feb 22, 2023 •

edited

Loading

simonjayhawkins commented Feb 22, 2023 •

edited

Loading

simonjayhawkins commented Feb 24, 2023 •

edited

Loading

topper-123 commented Mar 1, 2023 •

edited

Loading

jorisvandenbossche commented Mar 13, 2023 •

edited

Loading