[ python-Bugs-997050 ] ConfigParser behavior change

[SourceForge.net]

Bugs item #997050, was opened at 2004-07-24 08:30
Message generated for change (Comment added) made by goodger
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=997050&group_id=5470

Category: Python Library
Group: Python 2.4
Status: Open
Resolution: None
Priority: 5
Submitted By: David Goodger (goodger)
Assigned to: Fred L. Drake, Jr. (fdrake)
Summary: ConfigParser behavior change

Initial Comment:
ConfigParser.set() doesn't allow non-string arguments
for 'value' any
more.  This breaks Docutils code.  I submit that the
behavior should
not have been changed, rather that the documentation
needed updating.
I volunteer to undo the change and update the
documentation.

ConfigParser.py rev. 1.65 implements the fix detailed
in bug report
810843 (http://python.org/sf/810843):

    Fixed using methods (2) and (3): ConfigParser.set()
now checks
    that the value is some flavor of string, and raises
TypeError if
    it isn't.  This is documented behavior; regression
tests have been
    added.

"This is documented behavior": where was this behavior
documented
before the bug fix?  I couldn't find it.  If it wasn't
documented,
wouldn't method 4 (from the bug report, below) have
been more
appropriate?

    (4) Stating in the documentation that the raw parameter
    should be used when an option's value might not be of
    type string (this might be the intended usage, but it
    isn't made clear).

IOW, perhaps it wasn't a code bug but rather an
omission in the docs.
By adding the restriction and raising TypeError, it
breaks Docutils,
which subclasses ConfigParser.  If the string-only
restriction *was*
documented and I just missed it, I'll accept that the
Docutils code
was at fault (it's not the first time ;-) and rework
it.  But if that
restriction wasn't documented, I don't think
ConfigParser's behavior
should change.

See also
<http://article.gmane.org/gmane.text.docutils.devel/2073>.


----------------------------------------------------------------------

>Comment By: David Goodger (goodger)
Date: 2004-08-24 11:10

Message:
Logged In: YES 
user_id=7733

Also see the python-dev thread at
<http://mail.python.org/pipermail/python-dev/2004-August/046607.html>.
The third message in the thread was garbled due to a GPG
problem; reproduced below.

[Fred L. Drake, Jr.]
> David has (properly) been asking me to look into this, and
i've
> managed not to have enough time.  Sorry, David!

I thought a python-dev nudge might get you off your keister ;-)

> The ConfigParser documentation was certainly too vague,
but the
> problem, as I see it, is that the module was never
intended to store
> non-string values.

BUT ConfigParser *did* allow non-strings to be stored, for
internal
use only (can't write them out, of course).  And changing it
*did*
break code.  I think a doc change acknowledging that ability but
qualifying it ("for internal use only, can't be used to
write out
config files or interpolate values") would have been a
better fix.

The change in rev. 1.65 makes ConfigParser.py *less* useful.
 When I
used ConfigParser in Docutils, I wasn't being especially
clever when I
used it to set non-string values.  It will take a lot more
cleverness
post-rev-1.65 to enable that functionality.

Of course, the fix to Docutils is pretty simple: just override
ConfigParser.set with the older version.  But that depends
on private
implementation details (self._defaults, self._sections). 
This is
*much* more dangerous and impossible to future-proof against.

> I think allowing them is just asking for still more
trouble from
> that module down the road.

Has there been any trouble until now?  Why "fix" something
that wasn't
really broke?

The original bug report (http://www.python.org/sf/810843)
was really
only asking for clarification.  It ends with:

    Nonetheless, I was shocked to see what I thought was a
    straightforward use of ConfigParser throw off errors.

Imagine developers' shock now that we can't do what we want
at all!

Docutils is not the only project using ConfigParser to store
non-strings.  I wouldn't be surprised to see other code
breaking,
which we probably wouldn't find out about until after 2.4-final.

> Sure, the module could be made happy by reverting the
patch you
> cite, but happy-by-accident is a very fragile state to be in.

So let's remove the accident, and make the happiness official!

>> The comment for bug 810843 says "This is documented
behavior", but
>> I couldn't find any such documentation pre-dating this
change.
>
> This may have been a bug in my memory.

Now that the memory bug is fixed, how about reconsidering the
resultant decision?

I'll write a doc patch ASAP.

-- 
David Goodger <http://python.net/~goodger>

----------------------------------------------------------------------

Comment By: David Goodger (goodger)
Date: 2004-08-24 11:04

Message:
Logged In: YES 
user_id=7733

The alternative is the following change in Docutils. 
Unfortunately, it requires access to internal implementation
details marked with single leading underscores.

diff -u -r1.64 frontend.py
--- docutils/frontend.py	24 Jul 2004 14:13:38 -0000	1.64
+++ docutils/frontend.py	24 Aug 2004 15:01:32 -0000
@@ -637,6 +637,22 @@
                 section_dict[option] = self.get(section,
option, raw=1)
         return section_dict
 
+    def set(self, section, option, value):
+        """
+        Set an option.
+
+        Overrides stdlib ConfigParser's set() method to
allow non-string
+        values.  Required for compatibility with Python 2.4.
+        """
+        if not section or section == CP.DEFAULTSECT:
+            sectdict = self._defaults
+        else:
+            try:
+                sectdict = self._sections[section]
+            except KeyError:
+                raise CP.NoSectionError(section)
+        sectdict[self.optionxform(option)] = value
+
 
 class ConfigDeprecationWarning(DeprecationWarning):
     """Warning for deprecated configuration file features."""


----------------------------------------------------------------------

Comment By: David Goodger (goodger)
Date: 2004-08-24 11:00

Message:
Logged In: YES 
user_id=7733

The change that I'm proposing to revert:
http://cvs.sourceforge.net/viewcvs.py/python/python/dist/src/Lib/ConfigParser.py?r1=1.64&r2=1.65

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=997050&group_id=5470