Skip to content

[jit] Support for type annotations instead of torch.jit.annotate() #21390

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
driazati wants to merge 7 commits into from
Closed

[jit] Support for type annotations instead of torch.jit.annotate() #21390

driazati wants to merge 7 commits into from

Conversation

@driazati
Copy link
Contributor

@driazati driazati commented Jun 4, 2019 •
edited
Loading

This adds support for PEP 526 style annotations on assignments in place of
torch.jit.annotate(), so

a = torch.jit.annotate(List[int], [])

turns into

a : List[int] = []

Differential Revision: D15706021

[jit] Add support for type annotations
b1a8b6a 
This adds support for PEP 526 style annotations in place of
`torch.jit.annotate()`, so

```python
a = torch.jit.annotate(List[int], [])
a : List[int] = []
```
@pytorchbot pytorchbot added oncall: jit Add this issue/PR to JIT oncall triage queue module: pybind Related to our Python bindings / interactions with other Python libraries labels Jun 4, 2019
@driazati driazati changed the title [jit] Add support for type annotations [wip][jit] Add support for type annotations Jun 5, 2019
@ezyang ezyang added facebook and removed facebook labels Jun 5, 2019
@driazati driazati changed the title [wip][jit] Add support for type annotations [jit] Add support for type annotations Jun 6, 2019
@driazati driazati requested review from eellison and suo June 6, 2019 18:29
@driazati driazati changed the title [jit] Add support for type annotations [jit] Support for type annotations instead of torch.jit.annotate() Jun 6, 2019
@eellison
Copy link
Contributor

eellison commented Jun 6, 2019

Haven't looked very closely yet but could you add a test case for
x: List[int] = [0.5] ?
People will expect the annotations to have casting behavior but they don't

@driazati
Copy link
Contributor Author

driazati commented Jun 6, 2019

Added a test, mypy doesn't allow casting though so I don't think it's going to be expected

@eellison
Copy link
Contributor

eellison commented Jun 6, 2019

Added a test, mypy doesn't allow casting though so I don't think it's going to be expected

I've answered a question before where some one tried to do that, you'd be surpised

suo
suo previously requested changes Jun 7, 2019
View reviewed changes
torch/csrc/jit/script/tree_views.h
const Expr& rhs) {
return Assign(Compound::create(TK_ASSIGN, range, {lhs, rhs}));
}
static Assign create(
Copy link
Member

@suo suo Jun 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be Assign(Expr lhs, Expr rhs, Maybe<Expr> type) instead of two different constructors.

Also can you update the big comment at the top of the file with the new form?

@pytorchbot pytorchbot added the module: tests Issues related to tests (not the torch.testing module) label Jun 7, 2019
@driazati driazati dismissed suo’s stale review June 8, 2019 00:17

Addressed comments

@suo suo self-requested a review June 10, 2019 19:54
@yf225
Copy link
Contributor

yf225 commented Jun 11, 2019

This PR likely broke pytorch_linux_trusty_py3_5_test on master: https://circleci.com/gh/pytorch/pytorch/1969309?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link/console. Reverting.

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jun 12, 2019

@driazati merged this pull request in bbcd6cc.

@driazati driazati reopened this Jun 12, 2019
facebook-github-bot
facebook-github-bot reviewed Jun 12, 2019
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@driazati is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@kostmo
Copy link
Member

kostmo commented Jun 13, 2019

This change appears to have broken pytorch_linux_trusty_py3_5_test:

Jun 13 18:23:32 ======================================================================
Jun 13 18:23:32 ERROR: test_type_annotate_py3 (__main__.TestScript)
Jun 13 18:23:32 ----------------------------------------------------------------------
Jun 13 18:23:32 Traceback (most recent call last):
Jun 13 18:23:32   File "test_jit.py", line 3238, in test_type_annotate_py3
Jun 13 18:23:32     fn = get_fn('test_type_annotate_py3', script_path)
Jun 13 18:23:32   File "test_jit.py", line 182, in get_fn
Jun 13 18:23:32     spec.loader.exec_module(module)
Jun 13 18:23:32   File "<frozen importlib._bootstrap_external>", line 693, in exec_module
Jun 13 18:23:32   File "<frozen importlib._bootstrap_external>", line 799, in get_code
Jun 13 18:23:32   File "<frozen importlib._bootstrap_external>", line 759, in source_to_code
Jun 13 18:23:32   File "<frozen importlib._bootstrap>", line 222, in _call_with_frames_removed
Jun 13 18:23:32   File "/tmp/tmpxqv9gh0b/script.py", line 4
Jun 13 18:23:32     a : List[int] = []
Jun 13 18:23:32       ^
Jun 13 18:23:32 SyntaxError: invalid syntax

CC @yf225

@facebook-github-bot facebook-github-bot deleted the driazati/type_annot branch July 13, 2020 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@suo suo suo approved these changes

@eellison eellison Awaiting requested review from eellison

Assignees

No one assigned

Labels

Merged module: pybind Related to our Python bindings / interactions with other Python libraries module: tests Issues related to tests (not the torch.testing module) oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@driazati @eellison @yf225 @facebook-github-bot @kostmo @suo @ezyang @pytorchbot @mruberry