gh-66449: Add support to unnamed sections in ConfigParser by pslacerda · Pull Request #2735 · python/cpython (original) (raw)
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
OlegSchwann, johnlinp, ptman, kernc, septatrix, castro12321, ottok, martinschneider, arhadthedev, McSinyx, and 7 more reacted with thumbs up emoji
Hello, and thanks for your contribution!
I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).
Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.
Thanks again to your contribution and we look forward to looking at it!
This PR is stale because it has been open for 30 days with no activity.
This PR should be merged and not closed for being "stale".
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both test cases were successful:test_no_first_section (Lib.test.test_configparser.SectionlessTestCase) ... ok test_no_section (Lib.test.test_configparser.SectionlessTestCase) ... ok
SO examples work, but if you print the key you get DEFAULT and if you want to see the get() you need to use a blank key ''.
This PR is stale because it has been open for 30 days with no activity.
Why is this not merged yet? It would be great to have!
This PR is stuck for five years, so cc @ambv as a configparser expert.
This PR is stale because it has been open for 30 days with no activity.
@pslacerda It seems this PR got stalled because you didn't address the proposal to use a sentinel object instead of the empty string (as per your own suggestion).
Could you implement that change to get this moving again? Also, the branch would have be to rebased or main merged into it to get it up-to-date.
Would be great if we could finally get this completed!
@arhadthedev Good to read from you. Could you maybe help us to get this finally merged into main?
This PR is stale because it has been open for 30 days with no activity.
bpo-22253: Add support to unnamed sections in ConfigParser gh-66449: Add support to unnamed sections in ConfigParser
This comment was marked as abuse.
I give up on this, but not without publicly naming and shaming @ambv for not doing his job as the apparent maintainer of this part of the standard library.
Even with big desire, nobody can merge any new feature until 3.12b1 is out. The release date is already urgently moved from the last Monday to May 22. From https://discuss.python.org/t/postponing-3-12-beta-1-feature-freeze/26406:
Considering how many accepted changes are still being worked on (PEPs 684, 687, 688, 695, 697 at least) and still being decided on by the SC (PEPs 649, 702 and 709), I’m planning to postpone feature freeze and beta 1 for 3.12 by two weeks, without changing the rest of the schedule.
For the record: the postponement is just for the changes already in flight. Please don’t try to squeeze in more features because we have two more weeks. (Bugfixes are fine, of course.)
This PR is stale because it has been open for 30 days with no activity.
Python modules in the Lib dir
Stale PR or inactive for long period of time.
and removed stale
Stale PR or inactive for long period of time.
labels
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As well clarifying whether headerless options should leak into other sections, maybe there should be tests and documentation for the behaviour of:
- config.add_section(TOP_LEVEL)
- config[TOP_LEVEL] = ...
- Creating and retrieving TOP_LEVEL section when reading headerless section not enabled
- Using TOP_LEVEL as a regular name, e.g. sorted(config.keys())
| @@ -68,6 +69,12 @@ |
|---|
| converter gets its corresponding get*() method on the parser object and |
| section proxies. |
| When `allow_unnamed_section` is True (default: False), options |
| without section are accepted: the section for this is |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
options without a section?
| return "<TOP_LEVEL>" |
|---|
| def __eq__(self, other): |
| return repr(self) == repr(other) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did __eq__ get implemented? It makes more sense to me to keep the default inherited implementation. Doesn’t the class only get instantiated once anyway?
Won’t this make a valid section headed [<TOP_LEVEL>] match the headerless section in some cases?
| @@ -0,0 +1,4 @@ |
|---|
| :class:`configparser.ConfigParser` now accepts unnamed sections before named |
| ones, if configured to do so. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“accepts an unnamed section before named ones” might be clearer
| @@ -68,6 +69,12 @@ |
|---|
| converter gets its corresponding get*() method on the parser object and |
| section proxies. |
| When `allow_unnamed_section` is True (default: False), options |
| without section are accepted: the section for this is |
| ``configparser.UNNAMED_SECTION``. In the file, they are placed before |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configparser.TOP_LEVEL?
| @@ -18,7 +18,8 @@ |
|---|
| delimiters=('=', ':'), comment_prefixes=('#', ';'), |
| inline_comment_prefixes=None, strict=True, |
| empty_lines_in_values=True, default_section='DEFAULT', |
| interpolation=, converters=): |
| interpolation=, converters=, |
| allow_unnamed_section=False): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allow_unnamed_section not implemented
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems ok.
this feature would be very important to integrate. Is it available yet?
Note that a lot of the comments in the issue thread related to this are incorrect when they argue that this is not how INI files work.
The Windows INI files and the GetProfileString family of APIs have had this undocumented behavior forever, where an empty section is absolutely present and used by software.
See the following wine commits and bugs related to it:
wine-mirror/wine@be678a5
https://bugs.winehq.org/show_bug.cgi?id=8036
https://bugs.winehq.org/show_bug.cgi?id=18099
I would like to use ConfigParser, but currently cannot due to this missing feature, so I am open coding the config file parsing, which I would gladly avoid.
@vadmium Thanks for your review!
@pslacerda Are you still around and willing to make the changes suggested in the review? They seem quite reasonable to me.
@hw-claudio I alsolutely agree, it would be great to have!
Sadly it seems that it's a low-prio, or even a no-prio feature for Python's core developers 😢
I don't even remember the original motivation why I did this PR.
I failed to merge into this PR, so I did a new one.
Finally seems that we will get this code into main. Thank you @ChristianSi!
@pslacerda Wow, that's wonderful news, thanks a lot for all your effort!
Co-authored-by: Furkan Onder furkanonder@protonmail.com