Fix torch.[size|stride](dim=None)` invocation
#111991
Conversation
Per documentation, one should be able to explicitly pass dim argument as None to get tensor size across all dimentions/strides, but before this change it was incorrectly interpreted as named tensor call Add regression test to validate the behavior Fixes #111944
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/111991
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit df7888e with merge base a887ad0 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
malfet
added
release notes: python_frontend
test/test_misc.py
Outdated
| @@ -0,0 +1,17 @@ | |||
| # Owner(s): ["module: tests"] | |||
There was a problem hiding this comment.
I would put this into test/test_torch.py, that is where a lot of our basic tensor tests go
There was a problem hiding this comment.
I would agree that a test_misc is not very descriptive. Also no team is signed up to look at module: tests so I think we should avoid using it here as much as possible.
There was a problem hiding this comment.
Drumroll (this is where I got the idea in the first place):
Line 1 in ed15fa7
But sure, can move it to test_torch.py
test/test_misc.py
Outdated
| @@ -0,0 +1,17 @@ | |||
| # Owner(s): ["module: tests"] | |||
There was a problem hiding this comment.
I would agree that a test_misc is not very descriptive. Also no team is signed up to look at module: tests so I think we should avoid using it here as much as possible.
|
@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 mandatory check(s) failed. The first few are: Dig deeper by viewing the failures on hud |
|
@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 |
Per documentation, one should be able to explicitly pass dim argument as None to get tensor size across all dimentions/strides, but before this change it was incorrectly interpreted as named tensor call. Modify `size` and `stride` signatures generated by `gen_pyi.py` to highlight that overload with `None` will return a Tuple, but one with `dim: _int` returns `int`. Add regression test to validate the behavior, and remove the check for asserts from two named tensors tests (NamedTensors are dead, aren't they?) Fixes pytorch#111944 Pull Request resolved: pytorch#111991 Approved by: https://github.com/zou3519
Per documentation, one should be able to explicitly pass dim argument as None to get tensor size across all dimentions/strides, but before this change it was incorrectly interpreted as named tensor call. Modify `size` and `stride` signatures generated by `gen_pyi.py` to highlight that overload with `None` will return a Tuple, but one with `dim: _int` returns `int`. Add regression test to validate the behavior, and remove the check for asserts from two named tensors tests (NamedTensors are dead, aren't they?) Fixes pytorch#111944 Pull Request resolved: pytorch#111991 Approved by: https://github.com/zou3519
4 participants
Per documentation, one should be able to explicitly pass dim argument as None to get tensor size across all dimentions/strides, but before this change it was incorrectly interpreted as named tensor call.
Modify
sizeandstridesignatures generated bygen_pyi.pyto highlight that overload withNonewill return a Tuple, but one withdim: _intreturnsint.Add regression test to validate the behavior, and remove the check for asserts from two named tensors tests (NamedTensors are dead, aren't they?)
Fixes #111944