Streamlining guard expect tests #94404
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Edward Z. Yang <ezyang@meta.com> [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/94404
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 33d3071: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
github-actions
bot
requested review from
Chillee,
SherlockNoMad,
albanD,
antoniojkim,
bdhirsh,
jbschlosser and
miladm
Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]
This was referenced Feb 8, 2023
voznesenskym
reviewed
Feb 8, 2023
| # guards, and it may be helpful for user output too (be careful though; | ||
| # some equality guards are nontrivial! It would be nice to get simplified | ||
| # output to print them too) | ||
| def produce_guards(self, placeholders, sources, |
Collaborator
There was a problem hiding this comment.
I'm not sure a compilation protecting API should offer a simplified version that actively removes some of the safety. It feels like we should always emit them all, and if you need to remove simplified after, do so with a helper func.
Contributor
Author
There was a problem hiding this comment.
Can we cut the difference by making this a private kwarg?
Collaborator
There was a problem hiding this comment.
making it a private kwarg sounds good to me!
Changes: * Add `simplified` kwarg to let you only render guards that are nontrivial (excludes duck sizing) * Make a list of strings valid for sources, if you just have some variable names you want to bind to * Add test helper `show_guards` using these facilities, switch a few tests to it Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]
This was referenced Feb 8, 2023
Changes: * Add `simplified` kwarg to let you only render guards that are nontrivial (excludes duck sizing) * Make a list of strings valid for sources, if you just have some variable names you want to bind to * Add test helper `show_guards` using these facilities, switch a few tests to it Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]
Changes: * Add `simplified` kwarg to let you only render guards that are nontrivial (excludes duck sizing) * Make a list of strings valid for sources, if you just have some variable names you want to bind to * Add test helper `show_guards` using these facilities, switch a few tests to it Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]
Changes: * Add `simplified` kwarg to let you only render guards that are nontrivial (excludes duck sizing) * Make a list of strings valid for sources, if you just have some variable names you want to bind to * Add test helper `show_guards` using these facilities, switch a few tests to it Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]
Changes: * Add `simplified` kwarg to let you only render guards that are nontrivial (excludes duck sizing) * Make a list of strings valid for sources, if you just have some variable names you want to bind to * Add test helper `show_guards` using these facilities, switch a few tests to it Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]
Closed
Chillee
approved these changes
Feb 13, 2023
| # guards, and it may be helpful for user output too (be careful though; | ||
| # some equality guards are nontrivial! It would be nice to get simplified | ||
| # output to print them too) | ||
| def produce_guards(self, placeholders, sources, |
Collaborator
There was a problem hiding this comment.
making it a private kwarg sounds good to me!
ezyang
added a commit
that referenced
this pull request
Feb 14, 2023
CR on #94404 Signed-off-by: Edward Z. Yang <ezyang@meta.com> [ghstack-poisoned]
ezyang
added a commit
that referenced
this pull request
Feb 14, 2023
CR on #94404 Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]
ezyang
added a commit
to ezyang/pytorch
that referenced
this pull request
Feb 14, 2023
Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: f677f95 Pull Request resolved: pytorch#94404
pytorchmergebot
pushed a commit
that referenced
this pull request
Feb 15, 2023
CR on #94404 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: #94782 Approved by: https://github.com/voznesenskym
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack (oldest at bottom):
Changes:
simplifiedkwarg to let you only render guards that are nontrivial (excludes duck sizing)show_guardsusing these facilities, switch a few tests to itSigned-off-by: Edward Z. Yang ezyang@meta.com