Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConfigParser getters not available on SectionProxy #62359

Closed
jbvsmo mannequin opened this issue Jun 7, 2013 · 9 comments
Closed

ConfigParser getters not available on SectionProxy #62359

jbvsmo mannequin opened this issue Jun 7, 2013 · 9 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@jbvsmo
Copy link
Mannequin

jbvsmo mannequin commented Jun 7, 2013

BPO 18159
Nosy @ambv, @jbvsmo
Files
  • issue18159-3.diff: Implementation of type converters
  • 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:

    assignee = 'https://github.com/ambv'
    closed_at = <Date 2014-09-15.09:10:57.031>
    created_at = <Date 2013-06-07.17:43:50.063>
    labels = ['type-feature', 'library']
    title = 'ConfigParser getters not available on SectionProxy'
    updated_at = <Date 2014-09-15.09:10:57.021>
    user = 'https://github.com/jbvsmo'

    bugs.python.org fields:

    activity = <Date 2014-09-15.09:10:57.021>
    actor = 'python-dev'
    assignee = 'lukasz.langa'
    closed = True
    closed_date = <Date 2014-09-15.09:10:57.031>
    closer = 'python-dev'
    components = ['Library (Lib)']
    creation = <Date 2013-06-07.17:43:50.063>
    creator = 'JBernardo'
    dependencies = []
    files = ['30704']
    hgrepos = []
    issue_num = 18159
    keywords = ['patch']
    message_count = 9.0
    messages = ['190767', '190772', '190810', '191891', '193207', '216731', '226900', '226907', '226908']
    nosy_count = 3.0
    nosy_names = ['lukasz.langa', 'python-dev', 'JBernardo']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue18159'
    versions = ['Python 3.5']

    @jbvsmo
    Copy link
    Mannequin Author

    jbvsmo mannequin commented Jun 7, 2013

    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)

    @jbvsmo jbvsmo mannequin added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir labels Jun 7, 2013
    @ambv
    Copy link
    Contributor

    ambv commented Jun 7, 2013

    This is a reasonable feature request. Overriding __getattr__ doesn't look like the best solution, though. Let me think this over.

    @ambv ambv self-assigned this Jun 7, 2013
    @jbvsmo
    Copy link
    Mannequin Author

    jbvsmo mannequin commented Jun 8, 2013

    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.

    @ambv
    Copy link
    Contributor

    ambv commented Jun 25, 2013

    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.

    @ambv ambv added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Jun 25, 2013
    @jbvsmo
    Copy link
    Mannequin Author

    jbvsmo mannequin commented Jul 17, 2013

    Was the patch accepted yet? Looks good to me

    @jbvsmo
    Copy link
    Mannequin Author

    jbvsmo mannequin commented Apr 17, 2014

    ping?

    @ambv
    Copy link
    Contributor

    ambv commented Sep 15, 2014

    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.

    @ambv
    Copy link
    Contributor

    ambv commented Sep 15, 2014

    The new implementation also automatically covers get*() methods added on subclasses, no need to use converters= in that case.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 15, 2014

    New changeset 2c46a4ded259 by Łukasz Langa in branch 'default':
    Closes bpo-18159: ConfigParser getters not available on SectionProxy
    https://hg.python.org/cpython/rev/2c46a4ded259

    New changeset 5eb95d41ee43 by Łukasz Langa in branch 'default':
    Closes bpo-18159: ConfigParser getters not available on SectionProxy
    https://hg.python.org/cpython/rev/5eb95d41ee43

    @python-dev python-dev mannequin closed this as completed Sep 15, 2014
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant