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

shutil.which() docstring could be clearer #59353

Closed
tshepang mannequin opened this issue Jun 23, 2012 · 11 comments
Closed

shutil.which() docstring could be clearer #59353

tshepang mannequin opened this issue Jun 23, 2012 · 11 comments
Labels
docs Documentation in the Doc dir

Comments

@tshepang
Copy link
Mannequin

tshepang mannequin commented Jun 23, 2012

BPO 15148
Nosy @abalkin, @tarekziade, @bitdancer, @briancurtin, @hynek

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:

assignee = None
closed_at = <Date 2012-11-02.11:43:20.948>
created_at = <Date 2012-06-23.01:16:28.498>
labels = ['docs']
title = 'shutil.which() docstring could be clearer'
updated_at = <Date 2012-11-02.11:43:20.947>
user = 'https://bugs.python.org/tshepang'

bugs.python.org fields:

activity = <Date 2012-11-02.11:43:20.947>
actor = 'r.david.murray'
assignee = 'docs@python'
closed = True
closed_date = <Date 2012-11-02.11:43:20.948>
closer = 'r.david.murray'
components = ['Documentation']
creation = <Date 2012-06-23.01:16:28.498>
creator = 'tshepang'
dependencies = []
files = []
hgrepos = []
issue_num = 15148
keywords = []
message_count = 11.0
messages = ['163515', '163517', '163518', '163519', '163520', '163523', '163524', '163527', '163528', '174493', '174505']
nosy_count = 8.0
nosy_names = ['belopolsky', 'tarek', 'r.david.murray', 'brian.curtin', 'docs@python', 'tshepang', 'python-dev', 'hynek']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue15148'
versions = ['Python 3.3']

@tshepang
Copy link
Mannequin Author

tshepang mannequin commented Jun 23, 2012

I find this a little hard to parse (ignoring the obvious typo and the grammar error):

"""
Given a file, mode, and a path string, return the path whichs conform to the given mode on the path.
"""

One other suggestion: wouldn't 'file' read better as 'command'?

@tshepang tshepang mannequin assigned docspython Jun 23, 2012
@tshepang tshepang mannequin added the docs Documentation in the Doc dir label Jun 23, 2012
@tshepang tshepang mannequin changed the title shutul.which() docstring could be clearer shutil.which() docstring could be clearer Jun 23, 2012
@python-dev
Copy link
Mannequin

python-dev mannequin commented Jun 23, 2012

New changeset 5975292ddf82 by Alexander Belopolsky in branch 'default':
Issue bpo-15148: Fixed typos in shutil.which() docstring
http://hg.python.org/cpython/rev/5975292ddf82

@abalkin
Copy link
Member

abalkin commented Jun 23, 2012

*file* is correct because shutil.which() is more general than shell which command. (It can be used to find source files on PYTHONPATH, for example.)

I think the confusing part is "return the path ... on the path." This can be fixed in reST by marking the second "path" as the argument, but the docstring should be rephrased. Note that reST documentation for shutil.which() is missing in Doc/library/shutil.rst.

@briancurtin
Copy link
Member

I updated file to command in 973b4806f760. It needs to be command so it matches the implementation's argument name, and because it doesn't exactly take a file.

Alexander, can you explain the part about finding a file on PYTHONPATH? I don't think this has anything to do with shutil.which, or at least I have no idea how it would.

@abalkin
Copy link
Member

abalkin commented Jun 23, 2012

Alexander, can you explain the part about finding a file on PYTHONPATH?

I thought about something like this:

>>> shutil.which('shutil.py', os.F_OK, ':'.join(sys.path))
'/Users/sasha/Work/python-hg/py3k/Lib/shutil.py'

but win32 code seems to assume a search for a command.

@abalkin
Copy link
Member

abalkin commented Jun 23, 2012

Brian,

Did you intend to commit Tools/msi/msi.py in changeset 973b4806f760?

@briancurtin
Copy link
Member

No, reverting.

@bitdancer
Copy link
Member

Yeah, Brian had it as 'file' before, and I asked him to change it because it is confusing. 'file' sounds like a Python file object, which this is not. 'filename' would be OK, but 'cmd', as you note, is what it is really about.

One possibility for the path confusion is to capitalize the references to the system path, since both posix and windows capitalize it.:

"Given a name, mode, and PATH, return the path to the first file which conforms to the given mode found on the PATH, or None if there is no such file. mode defaults to os.F_OK|os.X_OK. PATH can be specified via the path argument, if not specified it defaults to the PATH set in the environment."

@python-dev
Copy link
Mannequin

python-dev mannequin commented Jun 23, 2012

New changeset 5f18d9d34f73 by Brian Curtin in branch 'default':
Fix bpo-15148. Make the shutil.which docstring more thorough
http://hg.python.org/cpython/rev/5f18d9d34f73

New changeset aa153b827d17 by Brian Curtin in branch 'default':
Fix bpo-15148. Capitalize PATH, hopefully leading to less confusion
http://hg.python.org/cpython/rev/aa153b827d17

@hynek
Copy link
Member

hynek commented Nov 2, 2012

Any reason why this is still open?

@bitdancer
Copy link
Member

Doesn't look like it.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
None yet
Development

No branches or pull requests

4 participants