bpo-33187: Document 3.9 changes to xml.etree.ElementInclude.include #20438
Conversation
Looks like the merging of bpo-33187 and bpo-20928 was racy, resulting in this change going undocumented.
|
cc @scoder as author of the relevant changes. Seems to me like we can skip-news if we backport this to 3.9, since this is about changes in Python 3.9. |
| @@ -816,16 +816,25 @@ Functions | |||
| loader fails, it can return None or raise an exception. | |||
|
|
|||
|
|
|||
| .. function:: xml.etree.ElementInclude.include( elem, loader=None) | |||
| .. function:: xml.etree.ElementInclude.include( elem, loader=None, base_url=None, \ | |||
| max_depth=DEFAULT_MAX_INCLUSION_DEPTH) | |||
There was a problem hiding this comment.
Not sure if it's better to use the (undocumented) named constant or the integer here. I wouldn't want the constant to be documented (it can't be changed, for example), and the actual value is probably best looked up through introspection (since it might change). So, just having a talking name here is probably best.
There was a problem hiding this comment.
Thanks for the spacing fixes!
By talking name, do you mean use lowercase and spaces? I couldn't really find an example in the docs to match against. (Also, feel free to make more changes directly if you find that efficient!)
There was a problem hiding this comment.
With "talking name", I meant a named constant whose name explains the purpose.
I otherwise changed my mind, though. It's better if it's clear in the docs what the maximum depth is, even if that duplicates the value from the code. We should show the integer value here. Users will look up the documentation way more often than we risk to change the default value.
There was a problem hiding this comment.
Sounds good, made the change!
|
Thanks @hauntsaninja for the PR, and @scoder for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9. |
|
Thanks |
|
GH-20722 is a backport of this pull request to the 3.9 branch. |
…H-20438) Looks like the merging of bpo-33187 and bpo-20928 was racy, resulting in this change going undocumented. (cherry picked from commit 301f0d4) Co-authored-by: Shantanu <[email protected]>
…ythonGH-20438) Looks like the merging of bpo-33187 and bpo-20928 was racy, resulting in this change going undocumented.
Looks like the merging of bpo-33187 and bpo-20928 was racy, resulting in
this change going undocumented.
https://bugs.python.org/issue33187