GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-102799: Let pydoc use the exception instead of sys.exc_info by iritkatriel · Pull Request #102830 · python/cpython Conversation
|
| 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 |
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.
Separate suggestions:
- Keep handling of two args in order. First first and second second. Handling first in middle of second is unnecessary confusion.
- 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.
| 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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
-
👍 1 reaction
| del sys.modules[key] | ||
| module = __import__(path) | ||
| except: | ||
| except BaseException as e: |
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.
I personally prefer 'err' to 'e'. I have no idea of any general consensus.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
-
👍 1 reaction
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assignedLabels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
3 participants
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.