Skip to content
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.

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

Clean up some type annotations in benchmarks/fastrnns #49946

Closed
wants to merge 1 commit into from

Conversation

r-barnes
Copy link
Contributor

Summary: Upgrades type annotations from Python2 to Python3

Test Plan: Sandcastle tests

Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Dec 29, 2020

💊 CI failures summary and remediations

As of commit 1f2e7ca (more details on the Dr. CI page):


  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-CircleCI failure(s)

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@codecov
Copy link

codecov bot commented Dec 30, 2020

Codecov Report

Merging #49946 (1f2e7ca) into master (cf45d65) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #49946      +/-   ##
==========================================
- Coverage   80.72%   80.72%   -0.01%     
==========================================
  Files        1909     1909              
  Lines      207051   207051              
==========================================
- Hits       167144   167138       -6     
- Misses      39907    39913       +6     

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

…ytorch#49946)

Summary:
Pull Request resolved: pytorch#49946

Upgrades type annotations from Python2 to Python3

Test Plan: Sandcastle tests

Differential Revision: D25717510

fbshipit-source-id: 0b9402ed296e6c0cdc91cda00b1245e1e9ece8f4
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D25717510

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 7d0eecc.

facebook-github-bot pushed a commit that referenced this pull request Jan 14, 2021
Summary:
Simply add missing `from typing import List, Tuple` and `from torch import Tensor`

Fixes regression introduced by #49946

Pull Request resolved: #50517

Reviewed By: gchanan

Differential Revision: D25908379

Pulled By: malfet

fbshipit-source-id: a44b96681b6121e61b69f960f81c0cad3f2a8d20
@mruberry
Copy link
Collaborator

Unlanding. Looks like this is causing build failures like this:

Jan 14 00:15:36 ImportError while loading conftest '/var/lib/jenkins/workspace/benchmarks/fastrnns/conftest.py'.
Jan 14 00:15:36 benchmarks/fastrnns/__init__.py:1: in <module>
Jan 14 00:15:36     from .cells import *
Jan 14 00:15:36 benchmarks/fastrnns/cells.py:25: in <module>
Jan 14 00:15:36     def lstm_cell(input: Tensor, hidden: Tuple[Tensor, Tensor], w_ih: Tensor,
Jan 14 00:15:36 E   NameError: name 'Tensor' is not defined

@facebook-github-bot
Copy link
Contributor

This pull request has been reverted by 2639f1d.

@r-barnes
Copy link
Contributor Author

@mruberry : Sorry about that. The diff passed all tests everywhere before I landed. Any suggestions for testing this more thoroughly during my fix-resubmit cycle?

@mruberry
Copy link
Collaborator

@r-barnes Running the edited code paths locally might have revealed the issue, but the failure in CI was masked by an infra failure, which we don't typically block on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants