-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
contextlib.closing documentation should use a new example #66944
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
Comments
urllib.request.urlopen() now always produces a context manager (either a HTTPResponse or addinfourl object). The example for contextlib.closing still uses urllib.request.urlopen as an example for the context manager wrapper, see
This is confusing users that now expect the object not to be a context manager, see:
Can a different example be chosen? |
It may be pretty hard to come up with a good example for closing in the stdlib that shouldn't just be turned into a context manager… There are some types where using it in a with statement is sensible, but unusual enough that you might want to make it explicit (maybe logging.Handler?), but does that make a good example? |
One exception that comes to mind is generators. According to bpo-13814, it is deliberate that you have to call close(), so perhaps someone could come up with an example based on that. |
The traditional examples (urllib, socket, StringIO) grow context manager support as soon as the need becomes apparent. So, that leaves only user-defined classes, third-party classes, generators, and cases where we already have a competing context manager that does something other than close. A reasonable candidate is an sqlite connection. It is a legitimate use case for contextlib.closing() since connections need to be closed after use, and because they already are context managers that do something different than closing (the commit or rollback transactions). A generator example would work, but closing generators is a rare and esoteric need (not many people even realize than you *can* close a generator), so this would make a bad example. A user defined class can add its own __enter__ and __exit__ to close if needed, so it isn't common to use contextlib.closing() on your own classes. The primary use case is for third-party classes where you don't control the source code and a closing context manager isn't present; however, those make for bad examples because they aren't in the standard library (i.e. aren't familiar) and because the better maintained modules are also adding context manager support. Out of all of these, only the sqlite connection example seems to be durable (won't go away) and familiar (in the standard library and not an exotic feature). That said, I think it would be best to stick with the simplest and most familiar possible examples even if they are no longer needed. The purpose of the example is to show how closing() works. We can use the surrounding text to indicate that the example is no longer necessary and that the most common remain uses are to add closing() behavior to third-party code you don't control (if you do control the code, then adding CM support directly to the class would be preferable). |
…ive purposes (#112198) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
…ustrative purposes (pythonGH-112198) (cherry picked from commit 9af9ac1) Co-authored-by: Ville Skyttä <ville.skytta@iki.fi> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
…ustrative purposes (pythonGH-112198) (cherry picked from commit 9af9ac1) Co-authored-by: Ville Skyttä <ville.skytta@iki.fi> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
…ustrative purposes (python#112198) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
…ustrative purposes (python#112198) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Possibly worth mentioning sqlite where the context manager doesn't close the object, you have to use contextlib.closing? Edit: Oh this was already mentioned:
|
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs
contextlib.closing
example is for illustration purposes #112198contextlib.closing
example is for illustrative purposes (GH-112198) #114458contextlib.closing
example is for illustrative purposes (GH-112198) #114459The text was updated successfully, but these errors were encountered: