[Python-checkins] r86976 - in python/branches/py3k: Doc/library/configparser.rst Doc/library/fileformats.rst Lib/configparser.py Lib/test/test_cfgparser.py Misc/NEWS

[Éric Araujo]

Hello,

> Author: lukasz.langa
> New Revision: 86976
> Log: Issue 10499: Modular interpolation in configparser
> 
> Modified: python/branches/py3k/Doc/library/configparser.rst
Is the module still backward compatible with the 3.1 version, modulo
fixed bugs?  I haven’t been able to follow closely all the great
improvements you’ve been making, and there has been a lot of movement in
the code, so I’m not sure.

Thanks for taking up configparser.  Maybe it will become so useful and
extensible that you’ll have to release it on PyPI for older Pythons :)

> Modified: python/branches/py3k/Doc/library/fileformats.rst
This looks like unrelated changes that slipped in the commit.  I use svn
status and svn diff all the time (actually a shell function to get a
colorized diff, not plain svn diff).  To commit I use a script based on
http://selenic.com/repo/hg/file/tip/hgeditor that opens up the commit
message file and the commit diff in two Vim splits.  It’s very handy.

> Modified: python/branches/py3k/Lib/configparser.py
>
>          Raise DuplicateSectionError if a section by the specified name
>          already exists. Raise ValueError if name is DEFAULT.
>          """
> -        if section == self._default_section:
> +        if section == self.default_section:
>              raise ValueError('Invalid section name: %s' % section)
I think it’s the only error message using %s instead of %r.  The quotes
added by %r typically help spot names with spaces (embedded or trailing)
and the like.

> +        options = list(d.keys())
> +        if raw:
> +            return [(option, d[option])
> +                    for option in options]
> +        else:
> +            return [(option, self._interpolation.before_get(self, section,
> +                                                            option, d[option],
> +                                                            d))
> +                    for option in options]
The list call seems unneeded.  Minor style point: I avoid such dangling
arguments that don’t read great, I prefer to go to a newline after the
opening paren:
               return [(option, self._interpolation.before_get(
                   self, section, option, d[option], d)) for
                   option in options]

>  class ConfigParser(RawConfigParser):
> [snip]
> +    def __init__(self, *args, **kwargs):
> +        super().__init__(*args, **kwargs)
> +        if self.__class__ is ConfigParser:
> +            warnings.warn(
> +                "The ConfigParser class will be removed in future versions."
> +                " Use SafeConfigParser instead.",
> +                DeprecationWarning, stacklevel=2
> +            )

Using __new__ may be clearer: no need to check for self.__class__, but
still works.  Would not help people that subclass a deprecated class,
though.

Cheers