Type _sympy/functions.py [1/n]#136205
Type _sympy/functions.py [1/n]#136205bobrenjc93 wants to merge 6 commits intogh/bobrenjc93/16/basefrom
Conversation
Signed-off-by: Bob Ren <bobren@fb.com> [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/136205
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 8b4a056 with merge base 7755176 ( FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Signed-off-by: Bob Ren <bobrenfb.com> I was chatting with jamesjwu about strategies to learn the code and he suggested adding types to some files. This stack of PRs adds types to _sympy/functions.py cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 [ghstack-poisoned]
|
Meta-comment: typing code that interacts with Sympy is often quite difficult, as Sympy is not very static type friendly. Something to be aware of; sometimes the juice is not worth the squeeze. |
aorenste
left a comment
There was a problem hiding this comment.
I'm not sure if this is meant to completely annotate the types in this file, but if it is you should remove the "mypy: allow-untyped-defs" from the top.
Signed-off-by: Bob Ren <bobrenfb.com> I was chatting with jamesjwu about strategies to learn the code and he suggested adding types to some files. This stack of PRs adds types to _sympy/functions.py cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 [ghstack-poisoned]
Signed-off-by: Bob Ren <bobrenfb.com> I was chatting with jamesjwu about strategies to learn the code and he suggested adding types to some files. This stack of PRs adds types to _sympy/functions.py cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 [ghstack-poisoned]
You can use the ghstack option |
That's what I did! But it seems like adding reviewers automatically promotes the PR from draft status to in review. |
torch/utils/_sympy/functions.py
Outdated
| from .numbers import int_oo | ||
|
|
||
|
|
||
| T = TypeVar("T", bound=SupportsFloat) |
There was a problem hiding this comment.
Unlikely this TypeVar should be public, should probably begin with _T ('in both variable and name)` so it's not exported.
Skylion007
left a comment
There was a problem hiding this comment.
Other than the nit: glad to see the type ignores disappear.
jamesjwu
left a comment
There was a problem hiding this comment.
Good to go, but do change T to be private so it's not exported
Signed-off-by: Bob Ren <bobrenfb.com> I was chatting with jamesjwu about strategies to learn the code and he suggested adding types to some files. This stack of PRs adds types to _sympy/functions.py cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 [ghstack-poisoned]
|
@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: trunk / win-vs2019-cuda12.1-py3 / build Details for Dev Infra teamRaised by workflow job |
Signed-off-by: Bob Ren <bobrenfb.com> I was chatting with jamesjwu about strategies to learn the code and he suggested adding types to some files. This stack of PRs adds types to _sympy/functions.py cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 [ghstack-poisoned]
|
@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 |
Signed-off-by: Bob Ren <bobren@fb.com> I was chatting with @jamesjwu about strategies to learn the code and he suggested adding types to some files. This stack of PRs adds types to _sympy/functions.py Pull Request resolved: pytorch#136205 Approved by: https://github.com/Skylion007, https://github.com/jamesjwu
Signed-off-by: Bob Ren <bobren@fb.com> Turns out older versions of python, in particular 3.8 shows errors that 3.12 doesn't. For posterity these are the steps I took to reproduce: ``` conda create -n py38 python=3.8 conda activate py38 pip install -r requirements.txt lintrunner init dmypy restart && lintrunner --all-files --take MYPY ``` Pull Request resolved: #136339 Approved by: https://github.com/Skylion007 ghstack dependencies: #136205
Stack from ghstack (oldest at bottom):
Signed-off-by: Bob Ren bobren@fb.com
I was chatting with @jamesjwu about strategies to learn the code and he suggested adding types to some files. This stack of PRs adds types to _sympy/functions.py
cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10