Skip to content

gh-102799: Let pydoc use the exception instead of sys.exc_info #102830

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

Merged
merged 2 commits into from
Mar 21, 2023
Merged

gh-102799: Let pydoc use the exception instead of sys.exc_info #102830

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db6a8a9
gh-102799: pydoc doesn't need to use sys.exc_info
iritkatriel Mar 19, 2023
84fd615
implement code review suggestions
iritkatriel Mar 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions Lib/pydoc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,9 @@ def synopsis(filename, cache={}):
class ErrorDuringImport(Exception):
"""Errors that occurred while trying to import something to document it."""
def __init__(self, filename, exc_info):
if not isinstance(exc_info, tuple):
assert isinstance(exc_info, BaseException)
exc_info = type(exc_info), exc_info, exc_info.__traceback__
self.filename = filename
self.exc, self.value, self.tb = exc_info
Copy link
Member

Choose a reason for hiding this comment

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

Separate suggestions:

  1. Keep handling of two args in order. First first and second second. Handling first in middle of second is unnecessary confusion.
  2. Handle two cases of second separately. Don't create tuple just to unpack. Unpacking makes it slightly more obvious that same attributes are defined, but pack/unpack makes it harder to see what object is assigned to each attribute.
Suggested change
if not isinstance(exc_info, tuple):
assert isinstance(exc_info, BaseException)
exc_info = type(exc_info), exc_info, exc_info.__traceback__
self.filename = filename
self.exc, self.value, self.tb = exc_info
self.filename = filename
if isinstance(exc_info, tuple):
self.exc, self.value, self.tb = exc_info
else:
assert isinstance(exc_info, BaseException)
self.exc = type(exc_info)
self.value = exc_info
self.tb = exc_info.__traceback__

I am tempted to think that no user will ever raise this exception. pydoc is only documented as a command-line app, not as importable library. But this is not idlelib. Can/should we emit a DeprecationWarning in the isinstance clause? It could then be removed someday. Since there is no doc of the module content, no changed notice is needed.


Expand All @@ -411,8 +414,8 @@ def importfile(path):
spec = importlib.util.spec_from_file_location(name, path, loader=loader)
try:
return importlib._bootstrap._load(spec)
except:
raise ErrorDuringImport(path, sys.exc_info())
except BaseException as e:
raise ErrorDuringImport(path, e)

def safeimport(path, forceload=0, cache={}):
"""Import a module; handle errors; return None if the module isn't found.
Expand Down Expand Up @@ -440,21 +443,20 @@ def safeimport(path, forceload=0, cache={}):
cache[key] = sys.modules[key]
del sys.modules[key]
module = __import__(path)
except:
except BaseException as e:
Copy link
Member

Choose a reason for hiding this comment

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

I personally prefer 'err' to 'e'. I have no idea of any general consensus.

# Did the error occur before or after the module was found?
(exc, value, tb) = info = sys.exc_info()
if path in sys.modules:
# An error occurred while executing the imported module.
raise ErrorDuringImport(sys.modules[path].__file__, info)
elif exc is SyntaxError:
raise ErrorDuringImport(sys.modules[path].__file__, e)
elif type(e) is SyntaxError:
# A SyntaxError occurred before we could execute the module.
raise ErrorDuringImport(value.filename, info)
elif issubclass(exc, ImportError) and value.name == path:
raise ErrorDuringImport(e.filename, e)
elif isinstance(e, ImportError) and e.name == path:
# No such module in the path.
return None
else:
# Some other error occurred during the importing process.
raise ErrorDuringImport(path, sys.exc_info())
raise ErrorDuringImport(path, e)
for part in path.split('.')[1:]:
try: module = getattr(module, part)
except AttributeError: return None
Expand Down