Skip to content

configparser.ConfigParser.clean and .update bugs #61024

Closed

Description

@wolfmanx
mannequin
BPO 16820
Nosy @ambv
Files
  • bug_configparser.py: Description and test code
  • bug_configparser_update_order.py: Exercise preservation/modification of key order for OrderedDict/ConfigParser
  • 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 2013-01-01.22:56:39.019>
    created_at = <Date 2012-12-30.17:12:53.083>
    labels = ['type-bug', 'library']
    title = 'configparser.ConfigParser.clean and .update bugs'
    updated_at = <Date 2013-01-02.00:03:42.110>
    user = 'https://bugs.python.org/wolfmanx'

    bugs.python.org fields:

    activity = <Date 2013-01-02.00:03:42.110>
    actor = 'python-dev'
    assignee = 'lukasz.langa'
    closed = True
    closed_date = <Date 2013-01-01.22:56:39.019>
    closer = 'lukasz.langa'
    components = ['Library (Lib)']
    creation = <Date 2012-12-30.17:12:53.083>
    creator = 'wolfmanx'
    dependencies = []
    files = ['28493', '28514']
    hgrepos = []
    issue_num = 16820
    keywords = []
    message_count = 11.0
    messages = ['178588', '178640', '178641', '178671', '178672', '178710', '178755', '178759', '178773', '178775', '178792']
    nosy_count = 3.0
    nosy_names = ['lukasz.langa', 'python-dev', 'wolfmanx']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue16820'
    versions = ['Python 3.2', 'Python 3.3']

    Activity

    1. wolfmanx commented on Dec 30, 2012

      wolfmanxmannequin
      MannequinAuthor

      configparser.ConfigParser.clean() always fails:

          >>> cfg = configparser.ConfigParser()
          >>> if not hasattr(configparser.ConfigParser, 'clear'):
          ...     configparser.ConfigParser.clear = configparser_clear_compat
          >>> cfg.clear() #doctest: +ELLIPSIS
          Traceback (most recent call last):
          ...
          ValueError: Cannot remove the default section.

      configparser.ConfigParser.update() overwrites all sections except DEFAULT instead of updating them.

      See attached test file-

    2. added
      stdlibStandard Library Python modules in the Lib/ directory
      type-bugAn unexpected behavior, bug, or error
      on Dec 30, 2012
    3. self-assigned this
      on Dec 31, 2012
    4. python-dev commented on Dec 31, 2012

      python-devmannequin
      Mannequin

      New changeset dc5adc08f1a8 by Łukasz Langa in branch '3.2':
      Fixes parser.clean() reported in issue bpo-16820.
      http://hg.python.org/cpython/rev/dc5adc08f1a8

      New changeset 4fc2fea807e6 by Łukasz Langa in branch '3.3':
      Merged parser.clean() fix (issue bpo-16820) from 3.2.
      http://hg.python.org/cpython/rev/4fc2fea807e6

      New changeset c6f9bc5a0cf1 by Łukasz Langa in branch 'default':
      Merged parser.clean() fix (issue bpo-16820) from 3.2 through 3.3.
      http://hg.python.org/cpython/rev/c6f9bc5a0cf1

    5. ambv commented on Dec 31, 2012

      @ambv
      Contributor

      Thanks for your report, Wolfgang. The clean() method is now fixed. The
      update() situation is more complicated, however.

      The mapping protocol defines that

          mapping.update({'a': 'b', 'c': 'd'})

      and

          mapping.update(a='b', c='d')

      are equivalent to

      mapping['a'] = 'b'
      mapping['c'] = 'd'
      

      For configparser we decided that setting a section on a parser is
      a destructive operation (e.g. it overwrites previous options). If the user
      finds this undesirable, she can either use parser['section'].update() or
      parser.read_dict() like you suggested.

      The bug here is that __setitem__ for the DEFAULTSECT should also clear options
      previously set on this section.

    6. python-dev commented on Dec 31, 2012

      python-devmannequin
      Mannequin

      New changeset 459a23083b66 by Łukasz Langa in branch '3.3':
      Fixes __setitem__ on parser['DEFAULT'] reported in issue bpo-16820.
      http://hg.python.org/cpython/rev/459a23083b66

      New changeset f6fb5a5748f0 by Łukasz Langa in branch 'default':
      Merged parser['DEFAULT'].__setitem__ fix (issue bpo-16820) from 3.3.
      http://hg.python.org/cpython/rev/f6fb5a5748f0

    7. ambv commented on Dec 31, 2012

      @ambv
      Contributor

      For the record, the bug that caused the following to be equivalent:

      parser['DEFAULT'] = {'option': 'value'}
      parser['DEFAULT'].update({'option': 'value'})

      has been fixed for 3.3.1+ only. This way it's going to be easier for users to reason about the fix ("it was broken in 3.2.0 - 3.3.0").

      Note that the bug only affected __setitem__ on the default section.

    8. wolfmanx commented on Dec 31, 2012

      wolfmanxmannequin
      MannequinAuthor

      Thanks, works for me.

      I only noted the discrepancy and did not give it much thought.
      I will just implement a merge method on top of read_dict.
      That gives me all options that could be desired :).

      However, after implementing the entire compatibility layer, I found one more issue:

      Using self.remove_section() changes the section order during an update.
      I would prefer that the section be cleared instead of removed in order to preserve the section order. Since OrderedDict does indeed preserve the key order during update, I think that this does not violate the Mapping Protocol.

      If this is not desired, just go ahead and close the issue.

      See also attached example bug_configparser_update_order.py:

      OrderedDict does not change the order of keys upon .update():

      >>> od = OrderedDict((('section1', {}), ('section2', {})))
      
          >>> list(od.keys())
          ['section1', 'section2']
      >>> od.update((('section1', {}), ('section3', {})))
      
          >>> list(od.keys())
          ['section1', 'section2', 'section3']

      But ConfigParser changes the order of sections upon .update():

      >>> cfg = configparser.ConfigParser()
      >>> cfg.update((('section1', {}), ('section2', {})))
      
          >>> cfg.sections()
          ['section1', 'section2']
      >>> cfg.update((('section1', {}), ('section3', {})))
      
          >>> cfg.sections()
          ['section2', 'section1', 'section3']
    9. ambv commented on Jan 1, 2013

      @ambv
      Contributor

      This is a fair point. Stay tuned.

    10. python-dev commented on Jan 1, 2013

      python-devmannequin
      Mannequin

      New changeset f580342b63d8 by Łukasz Langa in branch '3.3':
      configparser: preserve section order when using __setitem__ (issue bpo-16820)
      http://hg.python.org/cpython/rev/f580342b63d8

      New changeset a758f561a280 by Łukasz Langa in branch 'default':
      Merged section order preservation fix when using __setitem__ (issue bpo-16820)
      http://hg.python.org/cpython/rev/a758f561a280

    11. python-dev commented on Jan 1, 2013

      python-devmannequin
      Mannequin

      New changeset 6f0cee62f0c6 by Łukasz Langa in branch '3.2':
      configparser: preserve section order when using __setitem__ (issue bpo-16820)
      http://hg.python.org/cpython/rev/6f0cee62f0c6

      New changeset 2f5320497017 by Łukasz Langa in branch '3.3':
      Null-merged 3.2 section order preservation fix when using __setitem__ (issue
      http://hg.python.org/cpython/rev/2f5320497017

      New changeset 27b698395d35 by Łukasz Langa in branch 'default':
      Null-merged 3.2 section order preservation fix when using __setitem__ (issue
      http://hg.python.org/cpython/rev/27b698395d35

    12. ambv commented on Jan 1, 2013

      @ambv
      Contributor

      Breaking section order when using __setitem__ was a bug and as such was fixed in 3.2.4 and 3.3.1.

      In the DEFAULTSECT case above someone could potentially use the broken behaviour as a feature so that change was only made for 3.3.1+. In the section order case there is no such controversy.

    13. python-dev commented on Jan 2, 2013

      python-devmannequin
      Mannequin

      New changeset d5c45089df2d by Łukasz Langa in branch '3.3':
      Misc/NEWS updated to tell about bpo-14590 and bpo-16820
      http://hg.python.org/cpython/rev/d5c45089df2d

      New changeset 7938847b2641 by Łukasz Langa in branch '3.2':
      Misc/NEWS updated to tell about bpo-16820
      http://hg.python.org/cpython/rev/7938847b2641

    14. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    stdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions