Skip to content

bpo-23835: Enforce that configparser defaults are strings#2558

Merged
ambv merged 3 commits into
python:masterfrom
aragilar:bpo-23835
Aug 21, 2017
Merged

bpo-23835: Enforce that configparser defaults are strings#2558
ambv merged 3 commits into
python:masterfrom
aragilar:bpo-23835

Conversation

@aragilar

@aragilar aragilar commented Jul 4, 2017

Copy link
Copy Markdown
Contributor

This is the patch attached to https://bugs.python.org/issue23835, updated to master.

https://bugs.python.org/issue23835

@ambv ambv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, reading defaults should be done by callingself.read_dict(), then it will behave consistently with doing:

cp['DEFAULT'] = {1: 2.4}

@aragilar

Copy link
Copy Markdown
Contributor Author

Yeah, that seems more sensible (I originally wrote the patch against 2.7, which is why I didn't use read_dict). Do you want the new version in a new PR, or just replace the current PR with the new code? Also, where should I add a test, I couldn't find where I should add it last time I looked.

@ambv

ambv commented Jul 13, 2017

Copy link
Copy Markdown
Contributor

Replacing the current PR is totally fine. There are tests for configparser in Lib/test/test_configparser.py.

@aragilar

Copy link
Copy Markdown
Contributor Author

Done.

@aragilar

Copy link
Copy Markdown
Contributor Author

Ping?

Add assert statements to the new test.
@ambv ambv added the skip news label Aug 21, 2017
@ambv

ambv commented Aug 21, 2017

Copy link
Copy Markdown
Contributor

I'll add a NEWS.d entry in a separate PR.

@ambv
ambv merged commit 44e6ad8 into python:master Aug 21, 2017
@ambv

ambv commented Aug 21, 2017

Copy link
Copy Markdown
Contributor

Thanks! ✨ 🍰 ✨

GadgetSteve pushed a commit to GadgetSteve/cpython that referenced this pull request Sep 10, 2017
* Enforce that configparser defaults are strings
* Update test_configparser.py
@aragilar
aragilar deleted the bpo-23835 branch June 19, 2023 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants