classification
Title: ConfigParser's getboolean method is broken
Type: crash Stage: test needed
Components: Library (Lib) Versions: Python 2.7
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: lukasz.langa Nosy List: Felix.Laurie.von.Massenbach, eric.araujo, lukasz.langa, orsenthil
Priority: low Keywords:

Created on 2010-11-11 12:06 by Felix.Laurie.von.Massenbach, last changed 2010-11-13 10:38 by orsenthil. This issue is now closed.

Messages (10)
msg120943 - (view) Author: Felix Laurie von Massenbach (Felix.Laurie.von.Massenbach) Date: 2010-11-11 12:06
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...)
msg120944 - (view) Author: Felix Laurie von Massenbach (Felix.Laurie.von.Massenbach) Date: 2010-11-11 12:11
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
msg120948 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2010-11-11 13:12
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.
msg120951 - (view) Author: Felix Laurie von Massenbach (Felix.Laurie.von.Massenbach) Date: 2010-11-11 14:08
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
msg120953 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2010-11-11 14:22
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.
msg120990 - (view) Author: Felix Laurie von Massenbach (Felix.Laurie.von.Massenbach) Date: 2010-11-12 00:15
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'
msg120995 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2010-11-12 00:35
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.
msg121102 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-11-13 01:18
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.
msg121103 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2010-11-13 01:20
You think wrong. Try it.
msg121128 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-11-13 10:38
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.
History
Date User Action Args
2010-11-13 10:38:24orsenthilsetmessages: + msg121128
2010-11-13 01:20:44lukasz.langasetmessages: + msg121103
2010-11-13 01:18:20orsenthilsetnosy: + orsenthil
messages: + msg121102
2010-11-12 00:58:47lukasz.langasetstatus: open -> closed
2010-11-12 00:35:46lukasz.langasetresolution: wont fix
messages: + msg120995
2010-11-12 00:15:32Felix.Laurie.von.Massenbachsetmessages: + msg120990
2010-11-11 15:15:29eric.araujosetcomponents: + Library (Lib), - Extension Modules
stage: test needed
2010-11-11 15:15:16eric.araujosetnosy: + eric.araujo
2010-11-11 14:22:25lukasz.langasetmessages: + msg120953
2010-11-11 14:08:22Felix.Laurie.von.Massenbachsetmessages: + msg120951
2010-11-11 13:12:38lukasz.langasetpriority: normal -> low

messages: + msg120948
2010-11-11 12:52:34georg.brandlsetassignee: lukasz.langa

nosy: + lukasz.langa
2010-11-11 12:11:57Felix.Laurie.von.Massenbachsetmessages: + msg120944
2010-11-11 12:06:20Felix.Laurie.von.Massenbachcreate