Add Any return annotation to __getattr__ methods that return a union of types.
#150204
Conversation
…ion of types. Summary: Adds an `Any` return type annotation to `__getattr__` methods in `torch/_ops.py` that return a union of types. Attribute access returning a union of types can cause issues downstream because consumers would need to handle all of the possible types to make the type checker happy. This doesn't seem to matter today for mypy, presumably because `Any` is always inferred when a return type annotation is missing, but it still makes explicit what mypy is already doing implicitly. Test Plan: Reviewers: Subscribers: Tasks: Tags:
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/150204
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (4 Unrelated Failures)As of commit 6103902 with merge base 35ff508 ( FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following jobs failed but was present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
|
|
This is ready for review! The two test failures are unrelated as far as I can tell. |
|
@malfet do you have thoughts on this from the infra side? I'd like to avoid a situation where internal typing is more strict than OSS mypy resulting in frequent reverts of OSS PRs for breaking internal typing. So usually we'd require a test for these changes such that they are actually testable in open source. |
|
Thanks! @malfet Just to clarify, is this PR ok to merge as-is, or should I not add these annotations without completely annotating the file and adding it to mypy-strict.ini? |
If it passes CI, it's OK as is, as long as everyone understands that without enabling linter that actively checks for those, one should not expect that those annotations will stay there, someone can as easily remove them later by accident |
Ah, got it. Thanks! |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Merge failedReason: 1 jobs have failed, first few of them are: inductor / unit-test / linux-jammy-cpu-py3.12-gcc11-inductor-halide / build Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge -i |
Merge startedYour change will be merged while ignoring the following 3 checks: pull / linux-jammy-py3-clang12-executorch / build, pull / cuda12.4-py3.10-gcc9-sm75 / test (pr_time_benchmarks, 1, 1, linux.g4dn.metal.nvidia.gpu), trunk / macos-py3-arm64-mps / test (mps, 1, 1, macos-m2-15) Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
…ion of types. (pytorch#150204) Adds an `Any` return type annotation to `__getattr__` methods in `torch/_ops.py` that return a union of types. Attribute access returning a union of types can cause issues downstream because consumers would need to handle all of the possible types to make the type checker happy. This doesn't seem to matter today for mypy, presumably because `Any` is always inferred when a return type annotation is missing, but it still makes explicit what mypy is already doing implicitly. Pull Request resolved: pytorch#150204 Approved by: https://github.com/malfet
Adds an
Anyreturn type annotation to__getattr__methods intorch/_ops.pythat return a union of types. Attribute access returning a union of types can cause issues downstream because consumers would need to handle all of the possible types to make the type checker happy. This doesn't seem to matter today for mypy, presumably becauseAnyis always inferred when a return type annotation is missing, but it still makes explicit what mypy is already doing implicitly.