ConfigParser's getboolean method is broken

[FelixLaurievonMassenbach]

BPO	10387
Nosy	@orsenthil, @merwok, @ambv

^{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 2010-11-12.00:58:47.363>
created_at = <Date 2010-11-11.12:06:20.878>
labels = ['library', 'type-crash']
title = "ConfigParser's getboolean method is broken"
updated_at = <Date 2010-11-13.10:38:24.416>
user = 'https://bugs.python.org/FelixLaurievonMassenbach'

bugs.python.org fields:

activity = <Date 2010-11-13.10:38:24.416>
actor = 'orsenthil'
assignee = 'lukasz.langa'
closed = True
closed_date = <Date 2010-11-12.00:58:47.363>
closer = 'lukasz.langa'
components = ['Library (Lib)']
creation = <Date 2010-11-11.12:06:20.878>
creator = 'Felix.Laurie.von.Massenbach'
dependencies = []
files = []
hgrepos = []
issue_num = 10387
keywords = []
message_count = 10.0
messages = ['120943', '120944', '120948', '120951', '120953', '120990', '120995', '121102', '121103', '121128']
nosy_count = 4.0
nosy_names = ['orsenthil', 'eric.araujo', 'lukasz.langa', 'Felix.Laurie.von.Massenbach']
pr_nums = []
priority = 'low'
resolution = 'wont fix'
stage = 'test needed'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue10387'
versions = ['Python 2.7']

[FelixLaurievonMassenbach]

If the config file has a boolean formatted as either True or False, python raises an attribute error when doing str.lower() on it. In my code I've worked around this in the following way:

class MyConfigParser(ConfigParser.RawConfigParser):
    def getboolean(self, section, option):
        result = self.get(section, option)
        try:
            trues = ["1", "yes", "true", "on"]
            falses = ["0", "no", "false", "off"]
            if result in trues:
                return True
            if result in falses:
                return False
        except AttributeError as err:
            if str(err) == "\'bool\' object has no attribute \'lower\'":
                return result
            raise err

Felix

(p.s. first bug report, sorry if it's a bit of a mess...)

[FelixLaurievonMassenbach]

Oops, that was the broken first version. Let's try again:

class MyConfigParser(ConfigParser.RawConfigParser):
    def getboolean(self, section, option):
        result = self.get(section, option)
        try:
            trues = ["1", "yes", "true", "on"]
            falses = ["0", "no", "false", "off"]
            if result.lower() in trues:
                return True
            if result.lower() in falses:
                return False
        except AttributeError as err:
            if str(err) == "\'bool\' object has no attribute \'lower\'":
                return result
            raise err

Felix

[ambv]

Felix, thanks for your report! :)

I believe you misunderstood that all ConfigParser objects by design should hold *only strings* inside. The same problem would appear if you tried to write() configuration from a parser with booleans or numbers to a file. You should convert values to strings before putting them on the parser.

This is a design deficiency in RawConfigParser that it allows setting values to other types. If you would use SafeConfigParser you'd see that it doesn't let assigning anything other than strings as option values.

As much as we would love straightening it out (ensuring that values must be strings in RawConfigParsers), we can't do that because of backwards compatibility concerns.

As for holding strings internally, this is a design decision that may look strange at first but when you look at it from the consistency perspective, it's better: when you load values from an INI file parser can't tell whether some specific value should be considered boolean or a string. "yes" is a valid string and a valid boolean value for the parser. Which one to hold internally? We don't know. So we store strings and let the user decide when he's using get(). The same should happen when you put values into a parser on your own.

I hope that explains it.

[FelixLaurievonMassenbach]

Perhaps I don't understand fully, but I am reading, for example, "option = True" from a config file. When doing this getboolean raises:

AttributeError: 'bool' object has no attribute 'lower'

Is it intended that you cannot store "True" and "False" as values in the config file?

Apologies if I'm being dense.

Felix

[ambv]

No problem Felix. But look:

Python 2.7 (r27:82500, Sep 24 2010, 12:26:28)
[GCC 4.3.4 20090804 (release) 1] on cygwin
Type "help", "copyright", "credits" or "license" for more information.
>>> config = """
... [section]
... does_it_work = True
... is_it_broken = False
... """
>>> from ConfigParser import RawConfigParser
>>> from StringIO import StringIO
>>> parser = RawConfigParser()
>>> sio = StringIO(config)
>>> parser.readfp(sio)
>>> parser.sections()
['section']
>>> parser.get('section', 'does_it_work')
'True'
>>> parser.get('section', 'is_it_broken')
'False'
>>> parser.getboolean('section', 'does_it_work')
True
>>> parser.getboolean('section', 'is_it_broken')
False

So, reading configuration options from files definitely works. And as you see in the first get() pair, it stores values internally as strings.

[FelixLaurievonMassenbach]

Ok, so I understand the issue, but why doesn't the set method simply convert to a string?

>>> from ConfigParser import RawConfigParser
>>> from StringIO import StringIO
>>> parser = RawConfigParser()
>>> config = """
[section]
test = True
"""
>>> parser.readfp(StringIO(config))
>>> parser.get("section", "test")
'True'
>>> parser.getboolean("section", "test")
True
>>> parser.set("section", "test", True)
>>> parser.get("section", "test")
True
>>> parser.getboolean("section", "test")

Traceback (most recent call last):
  File "<pyshell#33>", line 1, in <module>
    parser.getboolean("section", "test")
  File "C:\Python27\lib\ConfigParser.py", line 361, in getboolean
    if v.lower() not in self._boolean_states:
AttributeError: 'bool' object has no attribute 'lower'

[ambv]

This is unfortunately a backwards compatibility concern. Originally it wasn't made so that set() converts to string or accepts only strings and when the developers realized this mistake, it was too late (there were programs using this misfeature). That's one of the reasons SafeConfigParser was created (hence the name).

As I said in my original answer, SafeConfigParser doesn't let you set() anything but a string and this is how it should have been from the start. But we won't break backwards compatibility now, it's far too late for that I'm afraid.

Storing non-string data was misused by some programs by using set() and get() (not getboolean() or getint() etc. but bare get()).

I'm closing this as Won't Fix. Thanks for your input, it's very much appreciated.

[orsenthil]

On Fri, Nov 12, 2010 at 12:35:49AM +0000, Łukasz Langa wrote:

This is unfortunately a backwards compatibility concern. Originally
it wasn't made so that set() converts to string or accepts only
strings and when the developers realized this mistake, it was too
late (there were programs using this misfeature). That's one of the

Where would coercing to str() for set methods result in failures or
backward compatibility problems? I think get() methods always return
strings.

[ambv]

You think wrong. Try it.

[orsenthil]

On Sat, Nov 13, 2010 at 01:20:45AM +0000, Łukasz Langa wrote:

You think wrong. Try it.

Okay, I get it. Coercing would be a bad idea in RawConfigParser
because there are cases where get method can have raw=True and
coercing would break those behaviors.

The way the OP expressed it, it looked like a bug to me.
Here is one way, the OP's concern can be resolved.

Index: Lib/configparser.py
===================================================================

--- Lib/configparser.py (revision 86441)
+++ Lib/configparser.py (working copy)
@@ -892,6 +892,8 @@
         """
         if value.lower() not in self.BOOLEAN_STATES:
             raise ValueError('Not a boolean: %s' % value)
+        if str(value) in self.BOOLEAN_STATES:
+            return self.BOOLEAN_STATES[str(value)]
         return self.BOOLEAN_STATES[value.lower()]

     def _validate_value_type(self, value):

But this seems specific to the special case as this bug is raised for.
I am personally, +0 for this too.