This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: ConfigParser cleanup, items() [needs tests]
Type: Stage:
Components: Library (Lib) Versions:
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: fdrake Nosy List: fdrake, gvanrossum, loewis, niemeyer
Priority: normal Keywords:

Created on 2002-04-17 10:24 by niemeyer, last changed 2022-04-10 16:05 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
python-2.3a0-ConfigParser.patch niemeyer, 2002-04-17 10:24
python-2.3a0-ConfigParser-walk.patch niemeyer, 2002-04-18 17:07
python-2.3a0-ConfigParser.patch nobody, 2002-06-22 04:06 Version 2, with integrated walk.
Messages (18)
msg10367 - (view) Author: Gustavo Niemeyer (niemeyer) * (Python committer) Date: 2002-04-17 10:24
The first patch fixes a bug, implements some speed 
improvements, some memory consumption improvements, 
enforces the usage of the already available global 
variables, and extends the allowed chars in option 
names to be very permissive. 
 
The second one, if used, is supposed to be applied 
over the first one, and implements a walk() 
generator method for walking trough the options of a 
section. 
msg10368 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2002-04-18 06:04
Logged In: YES 
user_id=21627

I'd like to see this split into even more parts: a patch
that supposedly has *no* semantic change (ie. the speed
improvements, memory consumption improvements, use of global
variables); a patch that changes behavior (please explain in
which ways); and the patch that implements walk (which
appears to be missing currently).
msg10369 - (view) Author: Gustavo Niemeyer (niemeyer) * (Python committer) Date: 2002-04-18 17:07
Logged In: YES 
user_id=7887

I'd rather explain here the patch that changes behavior, 
since it's pretty small. This line in the regular 
expression OPTCRE: 
 
r'(?P<option>[]\-[\w_.*,(){}]+)' 
 
was replaced by: 
 
r'(?P<option>[^:=\s]+)' 
 
So that instead of giving a range of characters which may 
be part of the option name, it just looks for the 
separator chars and spaces. This behavior is already used 
in the headers, and I haven't found any good reason to 
deny usage of other characters as option names. 
 
In the same regular expression, I've also replaced 
'[ \t]' by '\s', but this shouln't change the current 
behavior at all. 
 
About the walk patch, I have no idea why it isn't 
attached. I remember to have checked the ticked, and it 
was there. Anyway, I'm attaching it again. 
msg10370 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2002-04-22 17:55
Logged In: YES 
user_id=6380

I'm assigning this to Fred Drake, who has some opinions on
ConfigParser.
msg10371 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2002-05-30 16:06
Logged In: YES 
user_id=3066

Two comments:

- "section in self.__sections.keys()" can be written
  (still more efficiently) as "section in self.__sections",
  *but* the use of the sections() method is done to be
  subclass-friendly.  I'm not sure how important this is
  since I can't imagine anyone actually subclassing
  from such a mass of messy code.  Perhaps these
  could be modified to use the has_section() method,
  which is relatively new.

- MAX_INTERPOLATION_DEPTH was not "honored"
  in the code since it's really a safety net; there's no
  intention that client code be able to change it.  (The
  sanity of the value is not checked before use.)  I'm
  inclined to retain the use of the constant in the
  interpolation code, and add a comment above the
  constant the it exists for informational purposes only.

Otherwise, I'm fine with the patch.  ;-)
Sorry for the delay in responding to this.
msg10372 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2002-05-30 16:11
Logged In: YES 
user_id=3066

I should clarify:  By "retain the use of the constant" I
mean the constant numeric literal, not MAX_INTERPOLATION_DEPTH.
msg10373 - (view) Author: Gustavo Niemeyer (niemeyer) * (Python committer) Date: 2002-06-03 21:45
Logged In: YES 
user_id=7887

Hello Fred! 
 
- About has_section(), I thought about the possibility of subclassing it. But, 
besides agreeing with you on the messy code matter, other functions 
weren't (and still aren't) considering this possibility, so I doubt some one will 
be able to subclass it anyways. Anyway, I may change the patch to use 
whatever you suggest. 
 
- About MAX_INTERPOLATION_DEPTH, I understand your position of 
wanting "a safety net", but my intention of using it in the code was not to 
let the user change it, but to clarify and give consistence to the code. I think 
using literal constants in the code is not a good measure at all. If this was 
adopted, many modules would have to be changed (in a bad way) for 
consistence. With a quick look at the library, I was able to find smtpd.py, 
base64.py, binhex.py, cgi.py, cmd.py, and many others using similar purpose 
constants. You'll probably agree with me if you think about this more 
extensively. 
 
msg10374 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2002-06-13 01:42
Logged In: YES 
user_id=3066

Ken, can you recall our thinking on the use of the constant
for MAX_INTERPOLATION_DEPTH when this was first written?  I
think you were more involved in this than the rest of us at
the time.
msg10375 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2002-06-21 16:20
Logged In: YES 
user_id=3066

Ken appears non-responsive, re-assigning to me.
msg10376 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2002-06-21 17:22
Logged In: YES 
user_id=3066

I'd like to accept a variation on this patch.

Specifically, I'd like "section in self.sections()" tests to
become "sections in self.__sections" consistently; your
proposed patch is better than the original, but as long as
we're not supporting subclassing in this way, let's go ahead
and use the better-performing solution.

I'll accept the use of MAX_INTERPOLATION_DEPTH and your
changes to the interpolation loop, but the loop itself will
probably be re-written later, in response to SF bug #511737.

The walk() method will need documentation.

Can you prepare a new patch for this?

Thanks!
msg10377 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2002-06-22 01:38
Logged In: YES 
user_id=3066

Changed to a "bug" report even though it's a patch, so that
all the ConfigParser reports can show up together.
msg10378 - (view) Author: Gustavo Niemeyer (niemeyer) * (Python committer) Date: 2002-06-22 02:14
Logged In: YES 
user_id=7887

Thanks for your review Fred!  
  
I'll fix the patch as suggested, and work on a solution for the referred bug 
as well.   
msg10379 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2002-06-22 02:16
Logged In: YES 
user_id=3066

Thanks!  I can take care of the referred bug; I already know
how I want to solve that one.  Thanks!
msg10380 - (view) Author: Gustavo Niemeyer (niemeyer) * (Python committer) Date: 2002-06-22 04:09
Logged In: YES 
user_id=7887

Deal!! ;-) 
 
Here is the updated patch. 
 
Thanks! 
msg10381 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2002-07-03 04:08
Logged In: YES 
user_id=3066

Any objections to my renaming "walk" to "items" (or even
"section_items")?  "walk" makes me think of os.path.walk(),
which uses a callback; what the function returns is a list
of name-value pairs, though, which makes me think "items".

Other than this, I'm ready to commit.
msg10382 - (view) Author: Gustavo Niemeyer (niemeyer) * (Python committer) Date: 2002-07-03 19:45
Logged In: YES 
user_id=7887

No problems at all!!

Thank you!
msg10383 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2002-09-27 15:50
Logged In: YES 
user_id=3066

Added code cleanup and bugfix code in Lib/ConfigParser.py
revisions 1.45 and 1.38.10.3.

The new items() method was checked in as Lib/ConfigParser.py
1.46 and Doc/lib/libcfgparser.tex 1.23.  I still need to
write unit tests for this method, so this tracker item will
remain open for now.  Updated summary.
msg10384 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2002-10-25 20:44
Logged In: YES 
user_id=3066

Added tests of .items() in Lib/test/test_cfgparser.py
revision 1.17.
History
Date User Action Args
2022-04-10 16:05:13adminsetgithub: 36449
2002-04-17 10:24:37niemeyercreate