stdlib/xml: fix return types for toxml/toprettyxml methods #10061
Conversation
This comment has been minimized.
This comment has been minimized.
|
After fixing the dangling implementation lines, I realized that my attempt at the stubs was too naïve due to how The "correct" way to stub this wasn't immediately obvious to me, so I wrote a small test case ( |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Thanks! This looks good to me. We hit this issue quite often in typeshed (where we need three overloads even though it feels like we should be able to get away with two), but the test cases make it easier to verify that type checkers are making the correct inference here.
Just one question about some of the test cases -- looks to me like there might be some minor duplication right now?
test_cases/stdlib/check_xml.py
Outdated
| assert_type(document.toxml(), str) | ||
| assert_type(document.toxml(encoding=None), str) | ||
| assert_type(document.toxml(encoding="UTF8"), bytes) | ||
| if sys.version_info >= (3, 9): | ||
| assert_type(document.toxml(standalone=True), str) | ||
| assert_type(document.toxml(encoding="UTF8", standalone=True), bytes) |
There was a problem hiding this comment.
Aren't these cases already covered by the cases on lines 9-16?
There was a problem hiding this comment.
These are all meant to test toprettyxml; apparently I goofed that up at some point (probably when adding the version checks for 3.9). Will swap them to the correct method.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Fixes #10060.
This commit adds overloads for
xml.dom.minidom.Node.toxmlandxml.dom.minidom.Node.toprettyxmlto returnstrwhenencoding=Noneandbytesotherwise. I also added return types when evaluating in Python < 3.9, which the stubs previously lacked; AFAICT the same return types should be valid there.