-
-
Notifications
You must be signed in to change notification settings - Fork 30.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-33800: Fix default argument for parameter dict_type of ConfigParser/RawConfigParser #7494
Conversation
Doc/library/configparser.rst
Outdated
@@ -901,9 +901,6 @@ ConfigParser Objects | |||
converter gets its own corresponding :meth:`get*()` method on the parser | |||
object and section proxies. | |||
|
|||
.. versionchanged:: 3.1 | |||
The default *dict_type* is :class:`collections.OrderedDict`. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not remove historical notes.
Doc/library/configparser.rst
Outdated
@@ -917,6 +914,8 @@ ConfigParser Objects | |||
providing consistent behavior across the parser: non-string | |||
keys and values are implicitly converted to strings. | |||
|
|||
.. versionchanged:: 3.7 | |||
The default *dict_type* is :class:`dict`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion to avoid puzzlement: :class:`dict`, which is now ordered.
I agree! Thanks for the review Éric :) |
Doc/library/configparser.rst
Outdated
@@ -915,7 +918,8 @@ ConfigParser Objects | |||
keys and values are implicitly converted to strings. | |||
|
|||
.. versionchanged:: 3.7 | |||
The default *dict_type* is :class:`dict`. | |||
The default *dict_type* is :class:`dict`, since it now guarantees | |||
insertion order. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds a little funny to me. What do you think of preserves
or keeps
rather than guarantees
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like preserves too :)
Doc/library/configparser.rst
Outdated
Items present in *vars* no longer appear in the result. The previous | ||
behaviour mixed actual parser options with variables provided for | ||
interpolation. | ||
.. versionchanged:: 3.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you are addressing another problem here, one that is only in master. With this change, it won't be possible to auto backport this fix to 3.7. Suggest you include this in another PR if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly. I'll update the PR.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
I have made the requested changes; please review again |
Thanks for making the requested changes! @ned-deily: please review the changes made to this pull request. |
Thanks @andresdelfino for the PR, and @ned-deily for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7. |
GH-7542 is a backport of this pull request to the 3.7 branch. |
…er/RawConfigParser (pythonGH-7494) (cherry picked from commit 3b0b90c) Co-authored-by: Andrés Delfino <adelfino@gmail.com>
https://bugs.python.org/issue33800