[Python-checkins] cpython (3.2): Closes #11670: configparser read_file now iterates over f.

[Ezio Melotti]

Hi,

On 27/04/2011 19.14, lukasz.langa wrote:
> http://hg.python.org/cpython/rev/6f937d6369b6
> changeset:   69631:6f937d6369b6
> branch:      3.2
> parent:      69629:0ff8f6105827
> user:        Łukasz Langa<lukasz at langa.pl>
> date:        Wed Apr 27 18:10:05 2011 +0200
> summary:
>    Closes #11670: configparser read_file now iterates over f.
>
> files:
>    Doc/library/configparser.rst |  29 ++++++++++--
>    Lib/configparser.py          |   8 +-
>    Lib/test/test_cfgparser.py   |  54 ++++++++++++++++++++++++
>    3 files changed, 82 insertions(+), 9 deletions(-)
>
>
> diff --git a/Doc/library/configparser.rst b/Doc/library/configparser.rst
> --- a/Doc/library/configparser.rst
> +++ b/Doc/library/configparser.rst
> @@ -974,18 +974,37 @@
>
>      .. method:: read_file(f, source=None)
>
> -      Read and parse configuration data from the file or file-like object in
> -      *f* (only the :meth:`readline` method is used).  The file-like object
> -      must operate in text mode.  Specifically, it must return strings from
> -      :meth:`readline`.
> +      Read and parse configuration data from *f* which must be an iterable
> +      yielding Unicode strings (for example any file object).

I wouldn't use 'any' here. File objects opened in binary mode will yield 
byte strings.

>
>         Optional argument *source* specifies the name of the file being read.  If
>         not given and *f* has a :attr:`name` attribute, that is used for
>         *source*; the default is ``'<???>'``.
>
>         .. versionadded:: 3.2
> -         Replaces :meth:`readfp`.
> +         Replaces :meth:`readfp`.
> +
> +      .. note::
> +
> +         Prior to Python 3.2,

This note could be a versionchanged and be less verbose (e.g. the 
before/after could be replaced by something like "and use it as 
parser.read_file(readline_generator(f))").

>   :meth:`readfp` consumed lines from the file-like
> +         argument by calling its :meth:`~file.readline` method. For existing code
> +         calling :meth:`readfp` with arguments which don't support iteration,
> +         the following generator may be used as a wrapper around the file-like
> +         object::
>
> +             def readline_generator(f):
> +                 line = f.readline()
> +                 while line != '':

The "!= ''" can probably go.

> +                     yield line
> +                     line = f.readline()
> +
> +         Before::
> +
> +             parser.readfp(f)
> +
> +         After::
> +
> +             parser.read_file(readline_generator(f))
>
>      .. method:: read_string(string, source='<string>')
>
> diff --git a/Lib/configparser.py b/Lib/configparser.py
> --- a/Lib/configparser.py
> +++ b/Lib/configparser.py
> @@ -694,10 +694,10 @@
>       def read_file(self, f, source=None):
>           """Like read() but the argument must be a file-like object.
>
> -        The `f' argument must have a `readline' method.  Optional second
> -        argument is the `source' specifying the name of the file being read. If
> -        not given, it is taken from f.name. If `f' has no `name' attribute,
> -        `<???>' is used.
> +        The `f' argument must be iterable, returning one line at a time.
> +        Optional second argument is the `source' specifying the name of the
> +        file being read. If not given, it is taken from f.name. If `f' has no
> +        `name' attribute, `<???>' is used.
>           """
>           if source is None:
>               try:
> diff --git a/Lib/test/test_cfgparser.py b/Lib/test/test_cfgparser.py
> --- a/Lib/test/test_cfgparser.py
> +++ b/Lib/test/test_cfgparser.py
> @@ -1235,6 +1235,59 @@
>                       del section[default]
>           return cf_copy
>
> +
> +class FakeFile:
> +    def __init__(self):
> +        file_path = support.findfile("cfgparser.1")
> +        with open(file_path) as f:
> +            self.lines = f.readlines()
> +            self.lines.reverse()
> +
> +    def readline(self):
> +        if len(self.lines):
> +            return self.lines.pop()
> +        return ''
> +
> +
> +def readline_generator(f):
> +    """As advised in Doc/library/configparser.rst."""
> +    line = f.readline()
> +    while line != '':

Ditto.

> +        yield line
> +        line = f.readline()
> +
> +
> +class ReadFileTestCase(unittest.TestCase):
> +    def test_file(self):
> +        file_path = support.findfile("cfgparser.1")
> +        parser = configparser.ConfigParser()
> +        with open(file_path) as f:
> +            parser.read_file(f)
> +        self.assertTrue("Foo Bar" in parser)
> +        self.assertTrue("foo" in parser["Foo Bar"])

These should be 'assertIn'.

> +        self.assertEqual(parser["Foo Bar"]["foo"], "newbar")
> +
> +    def test_iterable(self):
> +        lines = textwrap.dedent("""
> +        [Foo Bar]
> +        foo=newbar""").strip().split('\n')
> +        parser = configparser.ConfigParser()
> +        parser.read_file(lines)
> +        self.assertTrue("Foo Bar" in parser)
> +        self.assertTrue("foo" in parser["Foo Bar"])

Ditto.

> +        self.assertEqual(parser["Foo Bar"]["foo"], "newbar")
> +
> +    def test_readline_generator(self):
> +        """Issue #11670."""
> +        parser = configparser.ConfigParser()
> +        with self.assertRaises(TypeError):
> +            parser.read_file(FakeFile())
> +        parser.read_file(readline_generator(FakeFile()))
> +        self.assertTrue("Foo Bar" in parser)
> +        self.assertTrue("foo" in parser["Foo Bar"])

Ditto.

> +        self.assertEqual(parser["Foo Bar"]["foo"], "newbar")
> +
> +
>   class CoverageOneHundredTestCase(unittest.TestCase):
>       """Covers edge cases in the codebase."""
>
> @@ -1338,5 +1391,6 @@
>           CompatibleTestCase,
>           CopyTestCase,
>           ConfigParserTestCaseNonStandardDefaultSection,
> +        ReadFileTestCase,
>           CoverageOneHundredTestCase,
>           )

Best Regards,
Ezio Melotti