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.

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

bpo-41737: [doc] add error handling example to file open section in IO tutorial #22818

Closed
wants to merge 5 commits into from

Conversation

iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Oct 20, 2020

Tutorial currently doesn't show that OSError is what should be caught for file open errors.

https://bugs.python.org/issue41737

Doc/tutorial/inputoutput.rst Outdated Show resolved Hide resolved
iritkatriel and others added 2 commits Oct 21, 2020
Co-authored-by: kj <28750310+Fidget-Spinner@users.noreply.github.com>
@taleinat
Copy link
Contributor

taleinat commented Nov 23, 2020

IMO this isn't a good fit in this part of the tutorial. When just introducing open(), adding three lines of error handling makes things much less clear.

Also, not every call to open() needs to handle OSError at the site of the call, so this isn't a case of a pattern that should recommend to always, or almost always, be used. Rather, error handling needs to be done differently in different contexts.

Note that this part of the tutorial does include a prominent link to the docs for open(), in which the second sentence is "If the file cannot be opened, an OSError is raised."

Having said that, this part of the tutorial does go into quite a bit of detail, and there can be room to mention error cases and handling. Adding mention later in this section that OSError is raised when errors happen, such as the file or parent dir not existing, seems like a good idea to me.

@iritkatriel
Copy link
Member Author

iritkatriel commented Nov 23, 2020

I agree. Let me rework this to add the error handling later on.

@iritkatriel iritkatriel changed the title bpo-41737: [doc] add try-except to file open example in IO tutorial bpo-41737: [doc] add error handling example to file open section in IO tutorial Nov 24, 2020
>>> try:
... f = open('workfile', 'w')
... except OSError as e:
... print('Cannot open file: ', e)
...
Copy link
Contributor

@taleinat taleinat Dec 22, 2020

Choose a reason for hiding this comment

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

Is this our recommended way of doing this? Don't we recommend using with open(...) as f:? I'm thinking that this example, while short, clear and concise, should show the recommended approach.

Copy link
Member Author

@iritkatriel iritkatriel Dec 22, 2020

Choose a reason for hiding this comment

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

That's a good point. At the same time, I usually prefer try-except blocks to be specific rather than around a large code block like the whole with and everything inside it. What is the recommended way to do open with error handling?

The reason I made this PR was because I felt that the OP of bpo-41737 wasn't clear that the API contract is OSError. Maybe this is not the way to clarify this point.

Copy link
Contributor

@taleinat taleinat Dec 22, 2020

Choose a reason for hiding this comment

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

What is the recommended way to do open with error handling?

Good question! Maybe the above, and then with f: ...? Note that the very next section recommends with open(...) as f: as "good practice".

Maybe this is not the way to clarify this point.

I think this is definitely the right place to show how things should be done, though perhaps after the paragraph introducing using a "with" statement with open().

Copy link
Member Author

@iritkatriel iritkatriel Dec 22, 2020

Choose a reason for hiding this comment

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

There are some examples here https://docs.python.org/3/tutorial/errors.html

@iritkatriel
Copy link
Member Author

iritkatriel commented Jul 5, 2021

I don't remember what this was about, but it seems stuck - closing as part of my spring clean.

@iritkatriel iritkatriel closed this Jul 5, 2021
@iritkatriel iritkatriel deleted the bpo41737 branch Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants