Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix #7422: autodoc: fails with ValueError when using autodoc_mock_imports #7431

Merged
merged 2 commits into from Apr 9, 2020
Merged

Fix #7422: autodoc: fails with ValueError when using autodoc_mock_imports #7431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b15b013
Fix #7422: autodoc: fails with ValueError when using autodoc_mock_imp…
tk0miya Apr 7, 2020
63d610d
Merge branch '3.0.x' into 7422_autodoc_mock_imports_causes_ValueError
tk0miya Apr 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES
View file
Open in desktop
Expand Up @@ -18,6 +18,7 @@ Bugs fixed

* #7428: py domain: a reference to class ``None`` emits a nitpicky warning
* #7438: C++, fix merging overloaded functions in parallel builds.
* #7422: autodoc: fails with ValueError when using autodoc_mock_imports

Testing
--------
Expand Down
13 changes: 11 additions & 2 deletions sphinx/util/inspect.py
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@
import warnings
from functools import partial, partialmethod
from inspect import ( # NOQA
Parameter, isclass, ismethod, ismethoddescriptor, unwrap
Parameter, isclass, ismethod, ismethoddescriptor
)
from io import StringIO
from typing import Any, Callable, Mapping, List, Tuple
Expand Down Expand Up @@ -116,6 +116,15 @@ def getargspec(func: Callable) -> Any:
kwonlyargs, kwdefaults, annotations)


def unwrap(obj: Any) -> Any:
"""Get an original object from wrapped object (wrapped functions)."""
try:
return inspect.unwrap(obj)
Copy link

Choose a reason for hiding this comment

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

How about providing some callback to unwrap to check if wrapped object (if there is any) id is equal to the object itself. Like (based on https://github.com/python/cpython/blob/master/Lib/inspect.py#L509):

def checker(f):
     return id(f) == id(f.__wrapped__)
inspect.unwrap(obj, stop = checker)

I guess that you may get ValueError for an object that is not mock and miss the real error.

Copy link
Contributor

Choose a reason for hiding this comment

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

@JanuszL ValueError is being thrown from https://github.com/python/cpython/blob/master/Lib/inspect.py#L524 and it's catching exactly the error you're looking for this to catch.

see: https://github.com/python/cpython/blob/master/Lib/inspect.py#L506

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, ValueError means "a cycle is encountered". It's a real error in this case. I think it is an abnormal case. So let's talk if we encountered.

Copy link

Choose a reason for hiding this comment

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

Sure. I was only thinking about the situation where the cycle is encountered and this is not due to mocking. In such a case it should really throw, so maybe there is an easier way to check if this is a moc or some valid object.

except ValueError:
# might be a mock object
return obj
Copy link
Contributor

Choose a reason for hiding this comment

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

@tk0miya this looks good, but maybe log DEBUG/INFO?

Suggested change
return obj
logger.debug('Giving up trying to unwrap %r', obj)
return obj

Copy link
Member Author

Choose a reason for hiding this comment

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

I doubt the log will be helpful. Do you want to see it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine not seeing it, I was just thinking it could be useful if you were already debugging.



def unwrap_all(obj: Any) -> Any:
"""
Get an original object from wrapped object (unwrapping partials, wrapped
Expand Down Expand Up @@ -217,7 +226,7 @@ def isattributedescriptor(obj: Any) -> bool:
return True
elif isdescriptor(obj):
# non data descriptor
unwrapped = inspect.unwrap(obj)
unwrapped = unwrap(obj)
if isfunction(unwrapped) or isbuiltin(unwrapped) or inspect.ismethod(unwrapped):
# attribute must not be either function, builtin and method
return False
Expand Down