Skip to content

Conversation

@wconstab
Copy link
Contributor

No description provided.

@wconstab wconstab requested a review from eellison July 23, 2020 20:35
@wconstab wconstab requested a review from apaszke as a code owner July 23, 2020 20:35
@wconstab wconstab self-assigned this Jul 23, 2020
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Jul 23, 2020
@wconstab wconstab force-pushed the wconstab/better_moduledict_error branch from 0d6608d to 203e62b Compare July 23, 2020 20:36
Copy link
Contributor

@eellison eellison left a comment

Choose a reason for hiding this comment

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

LGTM! Nice change

<< "Unable to extract string literal index. ModuleDict indexing is only supported with string literals.";
<< "Unable to extract string literal index. "
<< "ModuleDict indexing is only supported with string literals. "
<< "Enumeration of ModuleDict is supported, e.g. 'for m in self.values(): ...'";
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe for k, v in self.items() maybe bc it's more general

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.

@wconstab has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@wconstab merged this pull request in 646042e.

@wconstab wconstab deleted the wconstab/better_moduledict_error branch July 28, 2020 04:54
facebook-github-bot pushed a commit that referenced this pull request Sep 9, 2020
Summary:
Follow up to #41946, to suggest enumerating a module as an alternative if a user tries indexing into a modulelist/sequential with a non-integer literal

Pull Request resolved: #43361

Reviewed By: mrshenli

Differential Revision: D23602388

Pulled By: eellison

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

Labels

Merged oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants