GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-42179: Clarify exception chaining by greatvovan · Pull Request #23160 · python/cpython

Conversation

@greatvovan

@greatvovan greatvovan commented Nov 5, 2020 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Add an important detail and provide a link.

https://bugs.python.org/issue42179

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
greatvovan added 3 commits November 4, 2020 18:45
@greatvovan
Update errors.rst
86fe587 
Clarify exception chaining behaviour and give a reference to the library documentation.
@greatvovan
Update errors.rst
c29f3a2 
Wording
@greatvovan
Update errors.rst
35d34c3 
Spelling
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Nov 5, 2020
@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Nov 5, 2020
@greatvovan greatvovan changed the title bpo 42179: Clarify exception chaining bpo-42179: Clarify exception chaining Nov 5, 2020
@merwok

merwok commented Nov 5, 2020

Copy link
Copy Markdown
Member

I agree with @methane that this degree of detail is too much for the tutorial.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@greatvovan

greatvovan commented Nov 5, 2020

Copy link
Copy Markdown
Contributor Author

@merwok can you post (and elaborate) your opinion in the BPO ticket? I think it's better to keep discussion on the change in one place. I can reduce degree of detail, too.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
willingc
willingc requested changes Nov 5, 2020
View reviewed changes

@willingc willingc left a comment

Copy link
Copy Markdown
Contributor

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.

Thanks @greatvovan for the PR. Let's update this after #23162 lands. I like the sentence that I've flagged and think it would be helpful.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/tutorial/errors.rst Outdated
inside an exception handler or :keyword:`finally` section. Exception chaining
can be disabled by using ``from None`` idiom:
inside an exception handler or :keyword:`finally` section, however in this case
``__context__`` attribute of the exception is used. Chaining with

@willingc willingc Nov 5, 2020

Copy link
Copy Markdown
Contributor

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 recommend submitting this sentence right before the builtin exceptions sentence after #23162 lands. Then add "Chaining with ... takes priority."

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting changes and removed awaiting review labels Nov 5, 2020
@bedevere-bot

bedevere-bot commented Nov 5, 2020

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@greatvovan
Merge branch 'master' into bugfix/bpo-42179
bb29890
@greatvovan

greatvovan commented Nov 6, 2020

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again.

@willingc the sentence you suggested could not be used as is in the new variant of text as the context is bit lost after removing mentions of special attributes. I added a new abstract in the end of the section, please let me know what you think.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting change review and removed awaiting changes labels Nov 6, 2020
@bedevere-bot

bedevere-bot commented Nov 6, 2020

Copy link
Copy Markdown

Thanks for making the requested changes!

@willingc: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot
bedevere-bot requested a review from willingc November 6, 2020 20:19
greatvovan
greatvovan commented Nov 6, 2020
View reviewed changes
Comment thread Doc/tutorial/errors.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
merwok
merwok reviewed Nov 6, 2020
View reviewed changes
Comment thread Doc/tutorial/errors.rst

>>> def func():
... raise IOError
... raise ConnectionError

@merwok merwok Nov 6, 2020

Copy link
Copy Markdown
Member

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.

Nice! One OSError was missed on line 294

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
merwok
merwok reviewed Nov 6, 2020
View reviewed changes
Comment thread Doc/tutorial/errors.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@greatvovan
Update errors.rst
2831bc0 
Remove mentioning of special attributes as folks think it's too much for beginners.
willingc
willingc approved these changes Nov 9, 2020
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting change review labels Nov 9, 2020
@greatvovan

greatvovan commented Nov 9, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Please review the final version.

I removed my proposal paragraph about __cause__ and __context__ as nobody seems to be happy with it. I kept only changes about exception type (https://bugs.python.org/issue42179#msg380435).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
willingc
willingc approved these changes Nov 9, 2020
View reviewed changes

@willingc willingc left a comment

Copy link
Copy Markdown
Contributor

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.

Thanks @greatvovan and good catch on ConnectionError. @merwok If you are good with this as it stands, please merge. Thanks ☀️

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@github-actions

github-actions Bot commented Dec 16, 2020

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 5 days
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Dec 16, 2020
@willingc willingc added skip issue skip news and removed skip issue labels Dec 16, 2020
@willingc
willingc merged commit 3f9fe23 into python:master Dec 16, 2020
@bedevere-bot bedevere-bot removed the awaiting merge label Dec 16, 2020
@merwok

merwok commented Dec 16, 2020

Copy link
Copy Markdown
Member

Thanks for merging Carol!

FYI the commit message is messy due to github’s poor UI.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@willingc

willingc commented Dec 16, 2020

Copy link
Copy Markdown
Contributor

Sorry about the commit message. Thanks @merwok.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@greatvovan
greatvovan deleted the bugfix/bpo-42179 branch December 16, 2020 17:10
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@greatvovan @adorilson
bpo-42179: Clarify exception chaining (pythonGH-23160)
e498947 
* Update errors.rst

Clarify exception chaining behaviour and give a reference to the library documentation.

* Update errors.rst

Wording

* Update errors.rst

Spelling

* Update errors.rst

Remove mentioning of special attributes as folks think it's too much for beginners.
@hugovk hugovk mentioned this pull request Mar 6, 2022
Update errors.rst #28148
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merwok merwok merwok left review comments
@willingc willingc willingc approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@greatvovan @merwok @bedevere-bot @willingc @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.