[fx] Don't use __module__ to test if a function is bound from C++#75896
[fx] Don't use __module__ to test if a function is bound from C++#75896peterbell10 wants to merge 5 commits intogh/peterbell10/304/basefrom
Conversation
The new `test_public_bindings.py` test means `__module__` will be set correctly in future, even for functions bound from C++. Instead, just test directly that the function is of the `BuiltinFunctionType` which only passes for functions exported with the CPython API. [ghstack-poisoned]
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit c246a04 (more details on the Dr. CI page): Expand to see more💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
…rom C++" The new `test_public_bindings.py` test means `__module__` will be set correctly in future, even for functions bound from C++. Instead, just test directly that the function is of the `BuiltinFunctionType` which only passes for functions exported with the CPython API. [ghstack-poisoned]
…rom C++" The new `test_public_bindings.py` test means `__module__` will be set correctly in future, even for functions bound from C++. Instead, just test directly that the function is of the `BuiltinFunctionType` which only passes for functions exported with the CPython API. [ghstack-poisoned]
|
@pytorchbot merge this please |
|
Merge failed due to Command Raised by https://github.com/pytorch/pytorch/actions/runs/2204416663 |
|
cc @peterbell10 looks like this needs a rebase! |
…rom C++" The new `test_public_bindings.py` test means `__module__` will be set correctly in future, even for functions bound from C++. Instead, just test directly that the function is of the `BuiltinFunctionType` which only passes for functions exported with the CPython API. [ghstack-poisoned]
…rom C++" The new `test_public_bindings.py` test means `__module__` will be set correctly in future, even for functions bound from C++. Instead, just test directly that the function is of the `BuiltinFunctionType` which only passes for functions exported with the CPython API. [ghstack-poisoned]
|
@pytorchbot merge this |
|
Hey @peterbell10. |
This xfail'ed test was fixed by #75896 but didn't show up in CI because it's a slow test. Pull Request resolved: #76268 Approved by: https://github.com/ngimel
…5896) Summary: The new `test_public_bindings.py` test means `__module__` will be set correctly in future, even for functions bound from C++. Instead, just test directly that the function is of the `BuiltinFunctionType` which only passes for functions exported with the CPython API. Pull Request resolved: #75896 Approved by: https://github.com/albanD Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/7a80fc2ce7568212b4620ef014f9a70f2efc0772 Reviewed By: seemethere, osalpekar Differential Revision: D35874477 fbshipit-source-id: 450e86c4e08ac9a344d30d2e716e3a86e38f3d54
Summary: This xfail'ed test was fixed by #75896 but didn't show up in CI because it's a slow test. Pull Request resolved: #76268 Approved by: https://github.com/ngimel Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/1a7e43be141ce01469d7605075cb1008bf19abd7 Reviewed By: seemethere, osalpekar Differential Revision: D35874478 fbshipit-source-id: 5f5a8e4cf665fea273017691ff8d15c973178771
Stack from ghstack (oldest at bottom):
The new
test_public_bindings.pytest means__module__will be setcorrectly in future, even for functions bound from C++. Instead, just
test directly that the function is of the
BuiltinFunctionTypewhichonly passes for functions exported with the CPython API.