8000 Incorrect return type hint for torch.nonzero · Issue #51434 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

Incorrect return type hint for torch.nonzero #51434

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.

Sign up for GitHub

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

Jump to bottom
Closed
nlgranger opened this issue Jan 31, 2021 · 2 comments
Closed

Incorrect return type hint for torch.nonzero #51434

nlgranger opened this issue Jan 31, 2021 · 2 comments
Assignees
@rgommers
Labels
module: typing Related to mypy type annotations triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@nlgranger
Copy link
nlgranger commented Jan 31, 2021 •
edited by pytorch-probot bot
Loading

🐛 Bug

The type hint for torch.nonzero is incomplete, it only specifies Tensor.

To Reproduce

Steps to reproduce the behavior:

  1. Call torch.nonzero with as_tuple=True
a, b = torch.nonzero(torch.randn(10, 10), as_tuple=True)
  1. Run a static type checker such as pyright
  2. There is a warning

Expected behavior

There is no warning

Additional context

pytorch/tools/pyi/gen_pyi.py

Line 326 in 9dfbfe9

'def nonzero(input: Tensor, *, as_tuple: bool=...) -> Tensor: ...'],
should be like so:

                    'def nonzero(input: Tensor, *, as_tuple: bool=...) -> Union[Tensor, Tuple[Tensor, ...]]: ...'],

And Tensor.nonzero might need to be updated too I suppose.

cc @mruberry @rgommers @heitorschueroff @ezyang @malfet @xuzhao9 @gramster
@H-Huang H-Huang added module: numpy Related to numpy support, and also numpy compatibility of our operators module: typing Related to mypy type annotations triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Feb 2, 2021
@mruberry mruberry removed the module: numpy Related to numpy support, and also numpy compatibility of our operators label Feb 2, 2021
@rgommers
Copy link
Collaborator
rgommers commented Feb 2, 2021

Thanks for the report @nlgranger.

Tensor.nonzero is missing on purpose: #42998 (comment)

-> Union[Tensor, Tuple[Tensor, ...]]

That isn't quite right, returning unions like that will just cause problems further down the line where mypy doesn't know if it's a tensor or a tuple. Instead it should have an extra overload (Literal[True] and Literal[False])to disambiguate as_tuple=True and as_tuple=False.

@rgommers
Copy link
Collaborator
rgommers commented Feb 2, 2021

Mypy seems confused, it doesn't warn:

import torch

t = torch.randn(10, 10)
reveal_type(t)

# Annotated as Tensor, but gives Any rather than complaining
a, b = torch.nonzero(t, as_tuple=True)
reveal_type(a)
reveal_type(b)

c = torch.nonzero(t, as_tuple=True)
reveal_type(c)

d = torch.nonzero(t, as_tuple=False)
reveal_type(d)

gives:

$ mypy test/type_hint_tests/nonzero.py 
test/type_hint_tests/nonzero.py:4: note: Revealed type is 'torch.tensor.Tensor'
test/type_hint_tests/nonzero.py:7: note: Revealed type is 'Any'
test/type_hint_tests/nonzero.py:8: note: Revealed type is 'Any'
test/type_hint_tests/nonzero.py:11: note: Revealed type is 'torch.tensor.Tensor'
test/type_hint_tests/nonzero.py:14: note: Revealed type is 'torch.tensor.Tensor'

@rgommers rgommers self-assigned this Feb 3, 2021
rgommers added a commit to rgommers/pytorch that referenced this issue Feb 3, 2021
@rgommers
Fix torch.nonzero type annotation
9595d5b 
The overloads are a little tricky here. It's important that
the overloads are such that it's unambiguous what
`torch.nonzero(x)` will resolve to - so just specify defaults for
one of the overloads. Also, `out` is left out of the second overload
because a non-None value for `out` is not valid in combination with
`as_tuple=True`.

Closes pytorchgh-51434
@rgommers rgommers mentioned this issue Feb 3, 2021
Closed
xsacha pushed a commit to xsacha/pytorch that referenced this issue Mar 31, 2021
@rgommers
Fix torch.nonzero type annotation (pytorch#51635)
31109b9 
Summary:
The overloads are a little tricky here. It's important that the overloads are such that it's unambiguous what
`torch.nonzero(x)` will resolve to - so just specify defaults for one of the overloads. Also, `out` is left out of the second overload
because a non-None value for `out` is not valid in combination with `as_tuple=True`.

Closes pytorchgh-51434

Pull Request resolved: pytorch#51635

Reviewed By: zhangguanheng66

Differential Revision: D26279203

Pulled By: walterddr

fbshipit-source-id: 8459c04fc9fbf7fc5f31b3f631aaac2f98b17ea6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rgommers rgommers

Labels
module: typing Related to mypy type annotations triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix torch.nonzero type annotation rgommers/pytorch
4 participants
@rgommers @nlgranger @H-Huang @mruberry
0