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

[Style] Use typing_extensions.Self pre-3.11 Python #2487

Closed
eddiebergman opened this issue Aug 26, 2024 · 2 comments
Closed

[Style] Use typing_extensions.Self pre-3.11 Python #2487

eddiebergman opened this issue Aug 26, 2024 · 2 comments

Comments

@eddiebergman
Copy link
Contributor

eddiebergman commented Aug 26, 2024

Heyo,

I was browsing through some code and found this comment:

# 'Self', but at this point the verbose 'T...' syntax is needed.

Just thought to share that you can access this through from typing_extensions import Self. This will pass Mypy and LSP checks such as pylance in VSCode or others.

from typing_extensions import Self

     def fantasize(
        self,
        X: Tensor,
        sampler: MCSampler,
        observation_noise: Optional[Tensor] = None,
        **kwargs: Any,
    ) -> Self:

This is already a transitive dependancy of yours through torch

$ pip install botorch
$ pip install pipdeptree

$ pipdeptree
# Omitted
── torch [required: >=1.13.1, installed: 2.4.0]
    # Omitted
    └── typing_extensions [required: >=4.8.0, installed: 4.12.2]
pipdeptree==2.23.1
├── packaging [required: >=23.1, installed: 24.1]
└── pip [required: >=23.1.2, installed: 24.2]
setuptools==73.0.1
wheel==0.44.0

I did a quick check of other occurences and only found a similar usage in botorch/models/approximate_gp.py

$ git clone ...
$ cd botorch
$ rg TypeVar botorch

botorch/acquisition/input_constructors.py
16:from typing import Any, Callable, Optional, TypeVar, Union
108:T = TypeVar("T")

botorch/models/approximate_gp.py
35:from typing import Optional, TypeVar, Union
72:TApproxModel = TypeVar("TApproxModel", bound="ApproximateGPyTorchModel")

botorch/acquisition/logei.py
24:from typing import Callable, Optional, TypeVar, Union
68:FloatOrTensor = TypeVar("FloatOrTensor", float, Tensor)

botorch/models/model.py
19:from typing import Any, Callable, Optional, TYPE_CHECKING, TypeVar, Union
44:TFantasizeMixin = TypeVar("TFantasizeMixin", bound="FantasizeMixin")

If relevant: the license for typing_extensions is the same as Pythons

@eddiebergman eddiebergman changed the title [Style] Use typing_extensions.Self pre-3.1Python [Style] Use typing_extensions.Self pre-3.11 Python Aug 26, 2024
@pytorch pytorch deleted a comment Aug 26, 2024
@pytorch pytorch deleted a comment Aug 26, 2024
@esantorella
Copy link
Member

esantorella commented Aug 26, 2024

Thanks for the suggestion! Makes sense to me. Since typing_extensions is part of Python, I don't have any concerns about adding this dependency, and this seems to work with Pyre, which we use for type-checking, as well as with mypy.

This isn't going to be a high priority for the maintainers since we're just one minor version away from Python 3.11 and since this isn't broken now (just ugly) but I'd be happy to accept a pull request.

@eddiebergman
Copy link
Contributor Author

PR submitted #2494

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 a pull request may close this issue.

5 participants
@esantorella @eddiebergman and others