Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork10.9k
DOC: fixes classes decorated with set_module not showing its source on numpy's documentation (#28629)#28645
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
DOC: fixes classes decorated with set_module not showing its source on numpy's documentation (#28629)#28645
Uh oh!
There was an error while loading.Please reload this page.
Conversation
8bd6db3 to6541468Compareavoid calling stdlib module inspectTST: adds tests for set_module decoratorDOC: adds __file__ attribute to classes decorated with set_modulesimplify set_module and change attribute nameDOC : sets __module_file__ when overriding a __module__ in classesSpecity Exceptionsfix lint error
6f7127e to9e2f778Comparenumpy/_utils/__init__.py Outdated
| @@ -26,6 +27,12 @@ def example(): | |||
| """ | |||
| def decorator(func): | |||
| if module is not None: | |||
| if isinstance(func, type): | |||
| try: | |||
| func.__module_file__ = sys.modules.get(func.__module__).__file__ | |||
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.
Let's just call this_module_file. Giving it a dunder name is confusing because someone might think it's a name for a real dunder attribute that CPython supports. Also Python itself could add a dunder attribute with this name in the future. Another way to think about it is the language itself owns the namespace for symbols with dunder names, but leaves the single leading underscore namespace available for private use by Python libraries.
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.
Done.
Feel free to merge if you're happy but I'd love to take a look later today |
@melissawm and@ngoldbaum Sorry, but unfortunately it is not building the path in the right way! I will give another shot at fixing this later. |
Now it seems to be working. When building locally it points to the right file on themain branch, as expected. I checked it only for poly1d, so far. I don't now how to make a test for linkcode_resolve (should this be the case of a test?). Would like the commits to be squashed? |
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.
This looks good to me, nice! Unfortunately, I don't think numpy is set up to do a test docs run, so I don't know how to test it other than what you did, by building locally.
Some absolutely nitpicky comments in-line -- and might as well squash the commits when you do those. (But perhaps see first if there are other comments, and of course check that they don't break things...)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| @@ -26,6 +27,12 @@ def example(): | |||
| """ | |||
| def decorator(func): | |||
| if module is not None: | |||
| if isinstance(func, type): | |||
| try: | |||
| func._module_file = sys.modules.get(func.__module__).__file__ | |||
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.
Total nitpick, but justfunc._module_file = sys.modules[func.__module__].__file__ would be fine since you check forKeyError.
@melissawm As soon as you take a look and review I will squash the commits (and fetch, etc). |
melissawm left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Cool fix for an annoying problem! Tested locally and it works great. Thanks!
EDIT: Feel free to squash but we can do it for you when merging. Cheers!
Thanks@aureliobarbosa! |
e151f0d intonumpy:mainUh oh!
There was an error while loading.Please reload this page.
…n numpy's documentation (numpy#28629) (numpy#28645)* DOC: store file with set_module in classesavoid calling stdlib module inspectTST: adds tests for set_module decoratorDOC: adds __file__ attribute to classes decorated with set_modulesimplify set_module and change attribute nameDOC : sets __module_file__ when overriding a __module__ in classesSpecity Exceptionsfix lint error* rename variable* fix variable name* fix path
Uh oh!
There was an error while loading.Please reload this page.
This PR is intended tofix#28629
Classes decorated with the
_utils/set_moduledecorator can't be inspected for their source file, and thus the current strategy for getting the source for these classes using sphinx extension linkcode do not work. Currently 25 classes are affected. The currently problem doesn't affect functions decorated with set_module.This task should have been done in many ways (capturing
__file__,__module__, or via__code__). I choose to capture__file__because it allowed to fix the documentation with the inclusion of a very small code snipped on the functionlinkcode_resolve, used by linkcode to obtain the external link to numpy repository. It looked to me the capturing the original__module__would involve more code and many classes decorated with set_module were just empty, thus avoiding the possibility of capturing any__code__via function members.