classification
Title: Specify possible exceptions for os.chdir()
Type: enhancement Stage: resolved
Components: Documentation Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: aeros, docs@python, miss-islington, terry.reedy
Priority: normal Keywords: patch

Created on 2019-07-01 22:40 by aeros, last changed 2019-07-07 05:11 by terry.reedy. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 14611 merged aeros, 2019-07-05 22:08
PR 14629 merged miss-islington, 2019-07-07 01:20
PR 14630 merged miss-islington, 2019-07-07 01:20
PR 14631 merged terry.reedy, 2019-07-07 02:37
PR 14632 merged miss-islington, 2019-07-07 02:44
PR 14633 merged miss-islington, 2019-07-07 02:44
Messages (14)
msg347080 - (view) Author: Kyle Stanley (aeros) * (Python triager) Date: 2019-07-01 22:40
In the section describing the functionality of os.chdir(), there is no mention of possible errors that can be raised. This differentiates significantly from the sections of other methods, such as os.is_dir():

"This method can raise :exc:`OSError`, such as :exc:`PermissionError`, but :exc:`FileNotFoundError` is caught and not raised."

For the purpose of consistency and providing additional useful information, I would suggest doing the same for os.chdir(). os.chdir() differs from os.is_dir() by directly raising the OSErrors FileNotFoundError and NotADirectoryError rather than handling them. I would suggest the addition of the following in os.chdir()'s section:

"This method can raise :exc:`OSError`, such as exc:`FileNotFoundError` if the *path* does not lead to a valid file or exc:`NotADirectoryError` if the file specified is not a valid directory."

or a less verbose alternative:

"This method can raise :exc:`OSError`, such as exc:`FileNotFoundError` and exc:`NotADirectoryError`."

If a user is reading the documentation, they might naturally assume that os.chdir() catches the FileNotFoundError, similar to the behavior in os.is_dir(). Also, the NotADirectoryError is not explicitly mentioned anywhere else on the "os.rst" page. If it were to be mentioned anywhere, os.chdir() would probably be the most relevant location to do so.

Also, as a general question, what exactly is the standard for mentioning errors in the method sections for the Python documentation? For the purposes of clarity, I think it would be quite useful to explicitly mention errors, and a brief description of situations which raise them. If the behavior of the errors is consistent across of a group of methods, it can instead be mentioned in the header section. 

However, if the behavior not consistent, such as in this case, I think it is useful to briefly mention how the method deviates. It also seems useful to mention the error at least once if it is not explicitly mentioned elsewhere on the page, even if the parent is mentioned, such as with NotADirectoryError and OSError. This seems to be particularly important to do for the docs in a language such as Python, as it does not require the raise-able/throw-able errors to be mentioned in the code. 

I have already created a branch in my local fork of the cpython repository which implements this change, but I'll await feedback before submitting the PR.
msg347086 - (view) Author: Kyle Stanley (aeros) * (Python triager) Date: 2019-07-01 23:04
Minor clarifications:

This change is referring to the "Files and Directories" section of the "os.rst" page in "cpython/Doc/library/os.rst".

Also, I realized that I forgot to add the colon before "exc" in a couple locations. This was just a typo when writing the comment, it is correctly formatted in my local branch.
msg347371 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2019-07-05 19:33
> what exactly is the standard for mentioning errors
My impression is that there is none, or that it is inconsistent, but that the trend is to say more.  If you want to followup, check the documentation chapters of the devguide, and if there is nothing clear now, post to the pydev list.

https://docs.python.org/3/library/os.html#os-file-dir has

 os.chdir(path)
    Change the current working directory to path.
    This function can support specifying a file descriptor. The descriptor must refer to an opened directory, not an open file.

Most of the other entries in do not mention errors, but is_dir has the line mentioned.  I think the condensed line is enough to add, as the sub-exception names are self-explanatory; and when they occur, the full message certainly is.  (I verified both possibilities mentioned.)  Most users should just catch OSError, if anything.

Make a PR with the short message and request me as a reviewer.
msg347395 - (view) Author: Kyle Stanley (aeros) * (Python triager) Date: 2019-07-05 21:55
Thanks for the feedback terry. Do you think it would be helpful to work through some of the other commonly used functions in OS, adding condensed explanations of exceptions which can be raised? In general, it seems quite useful for users to be able to figure out the exceptions in which they may need to account for when using each of the functions. 

Also, should the naming convention be "method" or "function"? The two roughly refer to the same thing, but as far as I'm aware, the convention in Python is to refer to them as "functions". I'm  used to calling them "methods" from my prior experience with OOP, so that's why I did so in my initial condensed description. On the docs for OS, there seems to be a few instances where the term "method" is used. 

Personally, I don't mind the usage of either term, at the end of the day it's just semantics. But for the purposes of documentation, consistency seems preferable. Should I create a PR for replacing the mentions of "method" with "function"? Since it's quite minor and in the same OS.rst file, should I link it to this issue instead of creating a new one?
msg347399 - (view) Author: Kyle Stanley (aeros) * (Python triager) Date: 2019-07-05 22:14
Actually, I don't believe that I have the appropriate permissions to manually specify reviewers for PRs in the cpython repository. Do I have to be added a member of the Python organization on GitHub for this? I'm registered as a PSF contributing member, but I don't think that alone translates to any repository permissions. Could I be added as a member with low level permissions such as editing issue labels without being granted commit? This would also be helpful for adding "skip news" labels on any other issues, I've been doing a decent amount of PR reviews recently.
msg347450 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2019-07-06 22:22
Please ask your questions about doc conventions, both about exceptions and function/method.  Either someone will point to something in the devguide, or we might get a discussion about something that should be added.  I would not merge more extensive changes without some feedback.
msg347458 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2019-07-07 01:20
New changeset 0717b4d9b3899c5c2ca13031e4ff619a15a4d368 by Terry Jan Reedy (Kyle Stanley) in branch 'master':
bpo-37478: Specify possible exceptions for os.chdir() (GH-14611)
https://github.com/python/cpython/commit/0717b4d9b3899c5c2ca13031e4ff619a15a4d368
msg347459 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2019-07-07 02:18
New changeset 4e6bfc4c605d92d2395fbcded9cf45cdd1ced810 by Terry Jan Reedy (Miss Islington (bot)) in branch '3.8':
bpo-37478: Specify possible exceptions for os.chdir() (GH-14611) (GH-14629)
https://github.com/python/cpython/commit/4e6bfc4c605d92d2395fbcded9cf45cdd1ced810
msg347460 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2019-07-07 02:19
New changeset 1dd65075955337183ba2f78cb11a1eec2466dc74 by Terry Jan Reedy (Miss Islington (bot)) in branch '3.7':
bpo-37478: Specify possible exceptions for os.chdir() (GH-14611) (GH-14630)
https://github.com/python/cpython/commit/1dd65075955337183ba2f78cb11a1eec2466dc74
msg347462 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2019-07-07 02:44
New changeset a9b40e4546ca631e5ab41376b5b72e8f296f557d by Terry Jan Reedy in branch 'master':
bpo-37478: Add missing 'and'. (GH-14631)
https://github.com/python/cpython/commit/a9b40e4546ca631e5ab41376b5b72e8f296f557d
msg347463 - (view) Author: miss-islington (miss-islington) Date: 2019-07-07 02:49
New changeset e841a54206c65770aeb2b936cdc830dd4ed8bf9e by Miss Islington (bot) in branch '3.7':
bpo-37478: Add missing 'and'. (GH-14631)
https://github.com/python/cpython/commit/e841a54206c65770aeb2b936cdc830dd4ed8bf9e
msg347464 - (view) Author: miss-islington (miss-islington) Date: 2019-07-07 02:50
New changeset e414aa9cb002427a39dfd157cdad156336f93ca9 by Miss Islington (bot) in branch '3.8':
bpo-37478: Add missing 'and'. (GH-14631)
https://github.com/python/cpython/commit/e414aa9cb002427a39dfd157cdad156336f93ca9
msg347466 - (view) Author: Kyle Stanley (aeros) * (Python triager) Date: 2019-07-07 04:39
Exceptions for os.chdir() have been added to the docs.
msg347467 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2019-07-07 05:11
Spinoff issues should be separate.
History
Date User Action Args
2019-07-07 05:11:18terry.reedysetmessages: + msg347467
title: Docs: Method os.chdir() does not mention errors that can be raised -> Specify possible exceptions for os.chdir()
2019-07-07 04:39:04aerossetstatus: open -> closed
resolution: fixed
messages: + msg347466

stage: patch review -> resolved
2019-07-07 02:50:51miss-islingtonsetmessages: + msg347464
2019-07-07 02:49:42miss-islingtonsetnosy: + miss-islington
messages: + msg347463
2019-07-07 02:44:25miss-islingtonsetpull_requests: + pull_request14447
2019-07-07 02:44:18miss-islingtonsetpull_requests: + pull_request14446
2019-07-07 02:44:04terry.reedysetmessages: + msg347462
2019-07-07 02:37:13terry.reedysetpull_requests: + pull_request14445
2019-07-07 02:19:11terry.reedysetmessages: + msg347460
2019-07-07 02:18:53terry.reedysetmessages: + msg347459
2019-07-07 01:20:35miss-islingtonsetpull_requests: + pull_request14444
2019-07-07 01:20:27miss-islingtonsetstage: commit review -> patch review
pull_requests: + pull_request14443
2019-07-07 01:20:19terry.reedysetmessages: + msg347458
2019-07-06 22:22:53terry.reedysetassignee: docs@python -> terry.reedy
stage: patch review -> commit review
messages: + msg347450
versions: + Python 3.7, Python 3.8
2019-07-05 22:14:48aerossetmessages: + msg347399
2019-07-05 22:08:19aerossetkeywords: + patch
stage: patch review
pull_requests: + pull_request14425
2019-07-05 21:55:34aerossetmessages: + msg347395
2019-07-05 19:33:51terry.reedysetnosy: + terry.reedy
messages: + msg347371
2019-07-01 23:04:53aerossetmessages: + msg347086
2019-07-01 22:40:23aeroscreate