Skip to content
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

Use typing_extensions.Self in the stdlib #9694

Merged
merged 5 commits into from
Feb 9, 2023
Merged

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Feb 8, 2023

Only doing the stdlib for now, so that we don't have a repeat of #9690.

The first commit in this PR was prepared using the following script, which uses libcst:

from pathlib import Path

import libcst


class ModuleFixer(libcst.CSTTransformer):
    def leave_ImportFrom(self, original_node: libcst.ImportFrom, updated_node: libcst.ImportFrom) -> libcst.ImportFrom:
        if not (isinstance(original_node.module, libcst.Name) and original_node.module.value == "_typeshed"):
            return updated_node
        if isinstance(original_node.names, libcst.ImportStar):
            return updated_node
        new_imports = [
            item
            for item in original_node.names
            if not (isinstance(item, libcst.ImportAlias) and isinstance(item.name, libcst.Name) and item.name.value == "Self")
        ]
        if len(original_node.names) == 2 and len(new_imports) == 1:
            new_imports[0] = new_imports[0].with_changes(comma=libcst.MaybeSentinel.DEFAULT)
        return updated_node.with_changes(names=new_imports)

    def leave_Parameters(self, original_node: libcst.Parameters, updated_node: libcst.Parameters) -> libcst.Parameters:
        if not original_node.params:
            return updated_node

        first_param = original_node.params[0]
        if not (
            isinstance(first_param, libcst.Param)
            and isinstance(first_param.annotation, libcst.Annotation)
        ):
            return updated_node

        first_annotation = first_param.annotation.annotation
        if isinstance(first_annotation, libcst.Name):
            if first_annotation.value != "Self":
                return updated_node
        elif isinstance(first_annotation, libcst.Subscript):
            if not (
                isinstance(first_annotation.value, libcst.Name)
                and first_annotation.value.value == "type"
                and len(first_annotation.slice) == 1
                and isinstance(first_annotation.slice[0], libcst.SubscriptElement)
                and isinstance(first_annotation.slice[0].slice, libcst.Index)
                and isinstance(first_annotation.slice[0].slice.value, libcst.Name)
                and first_annotation.slice[0].slice.value.value == "Self"
            ):
                return updated_node
        else:
            return updated_node

        new_params = list(updated_node.params)
        new_params[0] = first_param.with_changes(annotation=None)
        return updated_node.with_changes(params=new_params)


def change_self_annotations(path: Path) -> None:
    source = path.read_text()
    if not source:
        return
    if "from _typeshed import" not in source:
        return
    if not (": Self" in source or ": type[Self]" in source):
        return
    sourcelines = source.splitlines()
    try:
        sourcelines.remove("from _typeshed import Self")
    except ValueError:
        pass
    first_non_comment_lineno = next(i for i, line in enumerate(sourcelines) if line.strip() and not line.strip().startswith("#"))
    sourcelines.insert(first_non_comment_lineno, "from typing_extensions import Self")
    cst = libcst.parse_module("\n".join(sourcelines) + "\n")
    new_source = cst.visit(ModuleFixer()).code
    path.write_text(new_source)


def main() -> None:
    for path in Path("stdlib").rglob("*.pyi"):
        print(f"Fixing '{path}'...")
        change_self_annotations(path)


if __name__ == "__main__":
    main()

Cf. #6300

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2023

Diff from mypy_primer, showing the effect of this PR on open source code:

mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ bson/son.py: note: In member "__new__" of class "SON":
+ bson/son.py:69: error: Value of type variable "Self" of "__new__" of "dict" cannot be "SON[_Key, _Value]"  [type-var]

prefect (https://github.com/PrefectHQ/prefect)
- src/prefect/utilities/pydantic.py:161: note:     def [Self] __new__(cls: Type[Self], str, Tuple[type, ...], Dict[str, Any], /, **kwds: Any) -> Self
+ src/prefect/utilities/pydantic.py:161: note:     def [_typeshed.Self] __new__(cls: Type[_typeshed.Self], str, Tuple[type, ...], Dict[str, Any], /, **kwds: Any) -> _typeshed.Self
- src/prefect/utilities/pydantic.py:163: note:     def [Self] __new__(cls: Type[Self], str, Tuple[type, ...], Dict[str, Any], /, **kwds: Any) -> Self
+ src/prefect/utilities/pydantic.py:163: note:     def [_typeshed.Self] __new__(cls: Type[_typeshed.Self], str, Tuple[type, ...], Dict[str, Any], /, **kwds: Any) -> _typeshed.Self

steam.py (https://github.com/Gobot1234/steam.py)
- steam/chat.py:401: note:     def [Self] __new__(cls: Type[Self], str, Tuple[type, ...], Dict[str, Any], /, **kwds: Any) -> Self
+ steam/chat.py:401: note:     def [_typeshed.Self] __new__(cls: Type[_typeshed.Self], str, Tuple[type, ...], Dict[str, Any], /, **kwds: Any) -> _typeshed.Self

@AlexWaygood AlexWaygood marked this pull request as ready for review February 8, 2023 18:48
@AlexWaygood
Copy link
Member Author

AlexWaygood commented Feb 8, 2023

mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ bson/son.py: note: In member "__new__" of class "SON":
+ bson/son.py:69: error: Value of type variable "Self" of "__new__" of "dict" cannot be "SON[_Key, _Value]"  [type-var]

Not sure what's going on in this one (source here), but it looks like a minor mypy bug in its handling of typing_extensions.Self. The other two mypy_primer hits are just noise caused by existing error messages changing slightly.

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primer hit is probably due to dict.__new__ now being incompatible with the bson type annotation. I'm fine with that.

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Feb 9, 2023

The primer hit is probably due to dict.__new__ now being incompatible with the bson type annotation. I'm fine with that.

It's weird that dict.__new__ has a different signature to dict.__init__ in our stubs. Maybe we should look into whether we actually need that.

@@ -196,9 +198,9 @@ _LiteralInteger = _PositiveInteger | _NegativeInteger | Literal[0] # noqa: Y026

class int:
@overload
def __new__(cls: type[Self], __x: str | ReadableBuffer | SupportsInt | SupportsIndex | SupportsTrunc = ...) -> Self: ...
def __new__(cls, __x: str | ReadableBuffer | SupportsInt | SupportsIndex | SupportsTrunc = ...) -> Self: ...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this change has introduced a new pyright issue at https://github.com/python/typeshed/blob/main/scripts/stubsabot.py#L37

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is that's probably a pyright issue in how it handles Self. What's the error message pyright produces?

Copy link
Collaborator

@Avasam Avasam Feb 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It thinks member is a int when previously it saw it as a ActionLevelSelf
image
image

vs before

image

I guess pyright doesn't take into account that the first classmethod parameter (cls) could be something else than it's own class.
mypy does it correctly:
image

Do you feel like opening a pyright issue or shall I?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yeah that seems like it might be a pyright bug (though Eric will be the judge of that). If you file an issue over there, he'll probably either fix it or triage it as "wontfix" within 24 hours :D

We also don't need the ActionLevelSelf TypeVar in stubsabot anymore, though, we can just use typing_extensions.Self there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also don't need the ActionLevelSelf TypeVar in stubsabot anymore, though, we can just use typing_extensions.Self there.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, created an issue here: microsoft/pyright#4668

@ShaneHarvey
Copy link
Contributor

Coming here because I'm attempting to fix or # type: ignore the SON error (in mongodb/mongo-python-driver#1194).

It's weird that dict.new has a different signature to dict.init in our stubs. Maybe we should look into whether we actually need that.

Has an issue been created for this yet? It's not clear to me whether this is a bug in pymongo or a bug in mypy.

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Apr 22, 2023

Coming here because I'm attempting to fix or # type: ignore the SON error (in mongodb/mongo-python-driver#1194).

It's weird that dict.new has a different signature to dict.init in our stubs. Maybe we should look into whether we actually need that.

Has an issue been created for this yet? It's not clear to me whether this is a bug in pymongo or a bug in mypy.

It looks like a mypy bug to me. The dict.__new__/dict.__init__ discrepancy in our stubs is strange, and it's very possible we could get rid of it, but I don't think it's the cause of this error. I'd suggest opening an issue over at mypy.

@ShaneHarvey
Copy link
Contributor

Thanks I opened: python/mypy#15135

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants