-
Notifications
You must be signed in to change notification settings - Fork 25.7k
TST Adds inplace checks to module_info #63739
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
Conversation
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 18244c0 (more details on the Dr. CI page):
1 failure not recognized by patterns:
🚧 1 fixed upstream failure:These were probably caused by upstream breakages that were already fixed.
Please rebase on the
|
Codecov Report
@@ Coverage Diff @@
## master #63739 +/- ##
==========================================
+ Coverage 66.60% 66.67% +0.07%
==========================================
Files 705 708 +3
Lines 92215 92412 +197
==========================================
+ Hits 61420 61619 +199
+ Misses 30795 30793 -2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update! Looking pretty good - couple minor comments below
test/test_modules.py
Outdated
| @modules(module_db) | ||
| def test_pickle(self, device, dtype, module_info): | ||
| # Test that module can be pickled and unpickled. | ||
| def test_check_inplace(self, device, dtype, module_info): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thomasjpfan Hm, looks like a merge conflict happened or something - we've now got 2 definitions for test_check_inplace that the linter is complaining about
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@jbschlosser has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@jbschlosser merged this pull request in 43c0f03. |
Follow up to #61935
This PR adds inplace checks to
test_modules. This version checks the constructor forinplaceand performs the check automatically.cc @albanD @mruberry @jbschlosser @walterddr