python / cpython

The Python programming language
https://www.python.org
Other
63.15k stars 30.24k forks source link

Update ConfigParser doc: accepts iterables, not just lists #71538

Closed 79fac99a-ad21-4657-8e1a-cde698759839 closed 4 years ago

79fac99a-ad21-4657-8e1a-cde698759839 commented 8 years ago
BPO 27351
Nosy @terryjreedy, @briancurtin, @ambv, @zhangyangyu, @ZackerySpytz, @iritkatriel
PRs
  • python/cpython#8123
  • python/cpython#9637
  • python/cpython#9638
  • 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: ```python assignee = 'https://github.com/ambv' closed_at = created_at = labels = ['3.7', '3.8', 'type-bug', 'library', 'docs'] title = 'Update ConfigParser doc: accepts iterables, not just lists' updated_at = user = 'https://bugs.python.org/RichRauenzahn' ``` bugs.python.org fields: ```python activity = actor = 'terry.reedy' assignee = 'lukasz.langa' closed = True closed_date = closer = 'terry.reedy' components = ['Documentation', 'Library (Lib)'] creation = creator = 'Rich.Rauenzahn' dependencies = [] files = [] hgrepos = [] issue_num = 27351 keywords = ['patch'] message_count = 10.0 messages = ['268838', '268839', '268840', '268914', '268915', '269215', '326683', '326685', '326686', '378278'] nosy_count = 7.0 nosy_names = ['terry.reedy', 'brian.curtin', 'lukasz.langa', 'Rich.Rauenzahn', 'xiang.zhang', 'ZackerySpytz', 'iritkatriel'] pr_nums = ['8123', '9637', '9638'] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue27351' versions = ['Python 3.6', 'Python 3.7', 'Python 3.8'] ```

    79fac99a-ad21-4657-8e1a-cde698759839 commented 8 years ago

    This came up on StackOverflow: http://stackoverflow.com/a/37903779/2077386

    I wanted to bring it to your attention in case it hasn't been notice before.

    It turns out that if you pass a fileobject (i.e., ConfigParser().read(open("foo"))) ConfigParser.read() will look at the argument, see it isn't a basestring, and then will iterate over it. fileobjects are iterables.

    This results in iterating over the contents of the file 'foo'. It then attempts to open a file named after every line read from 'foo'.

    For example, I made a file foo and filled it with a-g, each letter per line.

    strace shows:

    open("foo", O_RDONLY) = 3 open("a\n", O_RDONLY) = -1 ENOENT (No such file or directory) open("b\n", O_RDONLY) = -1 ENOENT (No such file or directory) open("c\n", O_RDONLY) = -1 ENOENT (No such file or directory) open("d\n", O_RDONLY) = -1 ENOENT (No such file or directory) open("e\n", O_RDONLY) = -1 ENOENT (No such file or directory) open("f\n", O_RDONLY) = -1 ENOENT (No such file or directory) open("g\n", O_RDONLY) = -1 ENOENT (No such file or directory)

    ...and since the API is designed to ignore files it can't open, it just ignores the open errors.

    I wonder if this fileobject case ought to be checked for when checking the arguments passed into ConfigParser.read().

    Thank you.

    zhangyangyu commented 8 years ago

    The doc tells ConfigParser.read accepts a filename or a list of filenames. Why do you pass it fileobject? If you want to use fileobject, why not read_file?

    79fac99a-ad21-4657-8e1a-cde698759839 commented 8 years ago

    Given that write() accepts a fileobject, but read() accepts a list of strings or a string (and readfp() is the one that accepts a fileobject instead), this seems like it could be a common enough error that just iterating over the fileobject could be undesirable and an exception might be thrown instead.

    I'm throwing this out here to see if the library maintainers were aware of this odd edge case.

    ambv commented 8 years ago

    Thanks for your report, Rich. configparser is one of the oldest libraries in the standard library. Its behavior might sometimes not be intuitive to newcomers but is burdened by years of backwards compatibility. Changing the behavior now to raise a type error on an open file passed to read would inevitably cause some program in the wild to break. Given this constraint, there's not much we can do besides documentation. If you have suggestions how we could improve it, let me know here.

    79fac99a-ad21-4657-8e1a-cde698759839 commented 8 years ago

    Thank you, lukasz. That's the answer I anticipated -- I can appreciate the backwards compatibility aspect very much.

    Regarding the docs, the docs say:

    "Attempt to read and parse a list of filenames, returning a list of filenames which were successfully parsed."

    I don't know if the convention in the docs is that list always means *list*, but it could be changed to be iterable since that is the implementation.

    That fileobjects are also iterables could be pointed out here, but I think anyone making this mistake isn't going to make the mistake from misreading the docs, it's from skipping the docs and assuming read() is consistent with write().

    terryjreedy commented 8 years ago

    Doing what the doc says is not a bug. The doc being obsolete in saying 'list' (which was once true) instead of the now correct 'iterable' is. The 3.6 ConfigParser.__doc___ also says 'list', so both docstrings and docs for all current versions should be fixed.

    briancurtin commented 6 years ago

    New changeset e45473e3ca31e5b78dc85cab575f5bb60d5b7f8f by Brian Curtin (Zackery Spytz) in branch 'master': bpo-27351: Fix ConfigParser.read() documentation and docstring (GH-8123) https://github.com/python/cpython/commit/e45473e3ca31e5b78dc85cab575f5bb60d5b7f8f

    briancurtin commented 6 years ago

    New changeset b0b8f9bd4e6f78ac7383b4e56cfb6cbacc77da89 by Brian Curtin (Miss Islington (bot)) in branch '3.7': bpo-27351: Fix ConfigParser.read() documentation and docstring (GH-8123) https://github.com/python/cpython/commit/b0b8f9bd4e6f78ac7383b4e56cfb6cbacc77da89

    briancurtin commented 6 years ago

    New changeset 3cd5e8e83c9785d9f505138903c7a50dc964101e by Brian Curtin (Miss Islington (bot)) in branch '3.6': bpo-27351: Fix ConfigParser.read() documentation and docstring (GH-8123) https://github.com/python/cpython/commit/3cd5e8e83c9785d9f505138903c7a50dc964101e

    iritkatriel commented 4 years ago

    This seems complete, can it be closed?