Skip to content

Conversation

@aragilar
Copy link
Contributor

@aragilar aragilar commented Jul 4, 2017

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

https://bugs.python.org/issue23835

Copy link
Contributor

@ambv ambv left a comment

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
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
Copy link
Contributor

ambv commented Jul 13, 2017

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

@aragilar
Copy link
Contributor Author

Done.

@aragilar
Copy link
Contributor Author

Ping?

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

ambv commented Aug 21, 2017

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

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

ambv commented Aug 21, 2017

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