classification
Title: ConfigParser getters not available on SectionProxy
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: lukasz.langa Nosy List: JBernardo, lukasz.langa, python-dev
Priority: normal Keywords: patch

Created on 2013-06-07 17:43 by JBernardo, last changed 2014-09-15 09:10 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
issue18159-3.diff lukasz.langa, 2013-06-25 23:21 Implementation of type converters review
Messages (9)
msg190767 - (view) Author: João Bernardo (JBernardo) Date: 2013-06-07 17:43
The configparser.RawConfigParser class implements some `get` methods: 
    get, getint, getfloat, getboolean

but if any of these get overridden on a subclass(with other arguments) or new ones are added (e.g. getlist), there's no way a SectionProxy instance will be able to use them.


    class DemoParser(ConfigParser):
        def getlist(self, section, option):
            return self.get(section, option).split()

    parser = DemoParser()
    parser.read(some_file)

    # These 2 lines should be equivalent, like "getint", but the
    # second doesn't work because of the SectionProxy instance
    parser.getlist('some_section', 'foo')
    parser['some_section'].getlist('foo')



Reading the code, for SectionProxy, it redefines all the get* methods from RawConfigParser, and that looks pretty bad...

A more elegant approach would be to fetch the function on the parser instance and bound to the section name with `lambda` or `functools.partial`... Something like:


    class SectionProxy(...):
        ...
        
        def __getattr__(self, attr):
            if not attr.startswith('get'):
                raise AttributeError(attr)
            fn = getattr(self._parser, attr)
            return lambda *args, **kw: fn(self._name, *args, **kw)
msg190772 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2013-06-07 19:57
This is a reasonable feature request. Overriding __getattr__ doesn't look like the best solution, though. Let me think this over.
msg190810 - (view) Author: João Bernardo (JBernardo) Date: 2013-06-08 17:39
> Overriding __getattr__ doesn't look like the best solution

Another idea would be to allow the proxy class to be selectable, but this would require the user to do much more coding for this simple thing...

I believe a proxy should be dynamic enough to avoid having to duplicate every function definition.
msg191891 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2013-06-25 23:21
There are several reasons why `get*()` methods are redefined on the section proxy. First of all, explicit is better than implicit. Secondly, the order of arguments is different: `parser.get()` has the fallback argument as the last (and keyword-only), whereas `parser['section'].get()` behaves like a mapping. You can do `parser['section'].get('key', 'some-fallback-value')` and `parser['section'].get('no-such-key')` returns None instead of raising `NoOptionError`.

This makes it difficult to automagically support parser `get*()` methods on the section proxy. This is why I decided to adapt a different mechanism: register a converter and a getter is automatically available:

  >>> cp = ConfigParser()
  >>> cp.converters['list'] = lambda value: value.strip().split()
  >>> cp.getlist('section', 'l')
  ['a', 'b', 'c']
  >>> cp['section'].getlist('l')
  ['a', 'b', 'c']
  >>> cp.getdict('section', 'd')
  Traceback (most recent call last):
  ...
  AttributeError: 'ConfigParser' object has no attribute 'getdict'
  >>> cp['section'].getdict('d')
  Traceback (most recent call last):
  ...
  AttributeError: 'ConfigParser' object has no attribute 'getdict'

This ensures that you can easily add new converters in subclasses or single instances and that the parser-level API and section-level API work like they should. This also makes implementing custom getters easier since there's no logic involved besides the conversion. And if you happen to need custom logic anyway, you can register a converter that is a callable class.
msg193207 - (view) Author: João Bernardo (JBernardo) Date: 2013-07-17 04:18
Was the patch accepted yet? Looks good to me
msg216731 - (view) Author: João Bernardo (JBernardo) Date: 2014-04-17 18:59
ping?
msg226900 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2014-09-15 07:17
The reason I didn't commit that patch before was that I wasn't sure whether making this change wouldn't create any unexpected backwards incompatibility. 

In fact, if I committed the patch as is, it would. I solved this by leaving getint, getfloat and getboolean on the parser class and keeping _get in use.
msg226907 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2014-09-15 09:08
The new implementation also automatically covers get*() methods added on subclasses, no need to use converters= in that case.
msg226908 - (view) Author: Roundup Robot (python-dev) Date: 2014-09-15 09:10
New changeset 2c46a4ded259 by Łukasz Langa in branch 'default':
Closes #18159: ConfigParser getters not available on SectionProxy
https://hg.python.org/cpython/rev/2c46a4ded259

New changeset 5eb95d41ee43 by Łukasz Langa in branch 'default':
Closes #18159: ConfigParser getters not available on SectionProxy
https://hg.python.org/cpython/rev/5eb95d41ee43
History
Date User Action Args
2014-09-15 09:10:57python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg226908

resolution: fixed
stage: patch review -> resolved
2014-09-15 09:08:25lukasz.langasetmessages: + msg226907
2014-09-15 07:17:42lukasz.langasetmessages: + msg226900
2014-09-15 06:52:02lukasz.langasetversions: + Python 3.5, - Python 3.4
2014-04-17 18:59:19JBernardosetmessages: + msg216731
2013-07-17 04:18:51JBernardosetmessages: + msg193207
2013-06-25 23:21:54lukasz.langasetfiles: + issue18159-3.diff
messages: + msg191891

keywords: + patch
type: behavior -> enhancement
stage: patch review
2013-06-08 17:39:54JBernardosetmessages: + msg190810
2013-06-07 19:57:22lukasz.langasetassignee: lukasz.langa
messages: + msg190772
2013-06-07 18:14:36r.david.murraysetnosy: + lukasz.langa
2013-06-07 17:43:50JBernardocreate