Issue 36322: Argument typo in dbm.ndbm.open (original) (raw)
Created on 2019-03-16 20:25 by rougeth, last changed 2022-04-11 14:59 by admin. This issue is now closed.
Messages (5)
Author: Marco Rougeth (rougeth) *
Date: 2019-03-16 20:25
Reading the documentation for dbm.gnu.open
I noticed that there were a typo in the flags
argument, it was documented as flag
, in plural form.
The same typo was present for dbm.ndbm.open
, but in this case, flag
makes more sense than flags
, since the function accepts only one option as a flag.
I opened a PR [1] fixing both typos, but I’d like to discuss if makes sense to rename the argument on dbm.ndbm.open
from flags
to flag
. As point out by @remilapeyre, this change would be backwards compatible, since we cannot use the function with keyword arguments.
dbm.ndbm.open(filename=‘foo’, flags=‘r’, mode=438) Traceback (most recent call last): File “”, line 1, in TypeError: open() takes no keyword arguments
What do you folks think about it?
1 - https://github.com/python/cpython/pull/12095
Author: Terry J. Reedy (terry.reedy) *
Date: 2019-03-23 00:02
I have trouble understanding this post. The parameter name is 'flag', not 'flags' and there are no typos, including in ndbm. The parameter name is singular because there is basically one flag with 4 possible values to indicate how to open. The fact that gdbm has optional subflags does not require changing the name and I think it better to keep the documented API the same.
The only time 'flags' appears is in "Not all flags are valid for all versions of gdbm. The module constant open_flags is a string of supported flag characters." Here, 'flags' refers to the multiple possible values for the 'flag' argument, not the argument itself.
I think that this issue and the PR should be closed.
Author: Marco Rougeth (rougeth) *
Date: 2019-03-23 12:12
Hi Terry, thanks for reviewing this and sorry for not being clear enough.
About dbm.gnu.open:
The docs indeed uses “flag”, in singular form, but it’s wrong because 1) the argument accepts, for some cases, 2 flags and, 2) the source code uses “flags” in plural form.
In [1]: import dbm.gnu
In [2]: help(dbm.gnu.open)
Help on built-in function open in module _gdbm:
open(filename, flags='r', mode=438, /) [...]
If you continue to read the docstring, there's an explanation about the cases where you can use two flags.
About dbm.ndbm.open:
For this one, the docs is also different from source code:
In [3]: import dbm.ndbm
In [4]: help(dbm.ndbm.open)
Help on built-in function open in module _dbm:
open(filename, flags='r', mode=438, /) [...]
The scope of the patch on Github ends here. It only makes the documentation consistent to the source code.
What I wanted to point out is that, in the case of ndbm.open, it accepts a flags option (in plural form) when it actually accepts only one. And since changing it would not make any difference from an user perspective, I believe we should go for it.
Author: Brett Cannon (brett.cannon) *
Date: 2019-04-02 21:21
Actually, it's even more subtle as the arguments are positional-only.
Author: Brett Cannon (brett.cannon) *
Date: 2019-04-02 21:25
So Terry is correct in so much as there is no parameter name. :) Both of the functions in question are positional-only, so the name actually doesn't matter beyond giving a way to reference the parameter in the documentation.
So thanks for the PR, Marco, but I'm going to reject this as there's nothing actually wrong with the gdbm or ndbm docs.
History
Date
User
Action
Args
2022-04-11 14:59:12
admin
set
github: 80503
2019-04-02 21:25:40
brett.cannon
set
status: open -> closed
resolution: rejected
messages: +
stage: patch review -> resolved
2019-04-02 21:21:07
brett.cannon
set
nosy: + brett.cannon
messages: +
2019-03-23 12:12:22
rougeth
set
messages: +
2019-03-23 00:02:46
terry.reedy
set
nosy: + terry.reedy
messages: +
2019-03-22 23:38:26
rougeth
set
keywords: + patch
stage: patch review
pull_requests: + <pull%5Frequest12453>
2019-03-16 22:31:41
rougeth
set
title: Argument typo in dam.ndbm.open -> Argument typo in dbm.ndbm.open
2019-03-16 20:27:24
remi.lapeyre
set
nosy: + remi.lapeyre
2019-03-16 20:25:09
rougeth
create