pytorch / botorch

Bayesian optimization in PyTorch
https://botorch.org/
MIT License
3.06k stars 390 forks source link

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

Open eddiebergman opened 2 weeks ago

eddiebergman commented 2 weeks ago

Heyo,

I was browsing through some code and found this comment: https://github.com/pytorch/botorch/blob/831072cacb1351adf4ac7cf0c5326a7437cbe2b5/botorch/models/model.py#L327

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

esantorella commented 2 weeks ago

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 commented 2 weeks ago

PR submitted #2494