typeshed icon indicating copy to clipboard operation
typeshed copied to clipboard

Use Element[Any] instead of Element in ElementTree

Open MatthewMckee4 opened this issue 7 months ago • 4 comments

Resolves #14036

MatthewMckee4 avatar May 30 '25 14:05 MatthewMckee4

Diff from mypy_primer, showing the effect of this PR on open source code:

archinstall (https://github.com/archlinux/archinstall)
- ./archinstall/tui/curses_menu.py:710: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
- https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
- Please report a bug at https://github.com/python/mypy/issues
- version: 1.15.0
- ./archinstall/tui/curses_menu.py:710: : note: use --pdb to drop into pdb
- Traceback (most recent call last):
-   File "mypy/checkexpr.py", line 5903, in accept
-   File "mypy/nodes.py", line 1889, in accept
-   File "mypy/checkexpr.py", line 3284, in visit_member_expr
-   File "mypy/checkexpr.py", line 3309, in analyze_ordinary_member_access
-   File "mypy/checkmember.py", line 207, in analyze_member_access
-   File "mypy/checkmember.py", line 226, in _analyze_member_access
-   File "mypy/checkmember.py", line 344, in analyze_instance_member_access
-   File "mypy/meet.py", line 96, in meet_types
-   File "mypy/subtypes.py", line 223, in is_proper_subtype
-   File "mypy/subtypes.py", line 351, in _is_subtype
-   File "mypy/types.py", line 1469, in accept
-   File "mypy/subtypes.py", line 581, in visit_instance
-   File "mypy/subtypes.py", line 2095, in infer_class_variances
-   File "mypy/subtypes.py", line 2057, in infer_variance
-   File "mypy/subtypes.py", line 186, in is_subtype
-   File "mypy/subtypes.py", line 351, in _is_subtype
-   File "mypy/types.py", line 3033, in accept
-   File "mypy/subtypes.py", line 1079, in visit_partial_type
- RuntimeError: Partial type "<partial list[?]>" cannot be checked with "issubtype()"

github-actions[bot] avatar May 30 '25 14:05 github-actions[bot]

I haven't looked too deeply into this, but I don't think changing the Element instances from Element (which is Element[str]) to just Element[Any] is the right course of action. This will probably make previous type safe code unsafe. For example:

from xml.etree import ElementTree

tree = ElementTree.parse("foo.xml")
el = tree.find(".//*")
assert el is not None
print(type(el.tag))

el.tag is str in this case and it's typed as such. Now, it is Any.

This needs a more nuanced approach. I'm not sure what problems @JelleZijlstra alluded to in #14036, though.

srittau avatar May 30 '25 14:05 srittau

The problem is primarily in argument types, not return types. For example, the tostring function is (still with this PR) marked as accepting only Element, which means only Element[str]. And because Element is invariant, that means any other Element is not allowed.

For return types, returning Element[str] might be fine though it should be verified that that's what these methods actually return.

I'm becoming skeptical that the way we're using TypeVar defaults has been a good idea; it can lead to usability issues quite easily.

JelleZijlstra avatar May 30 '25 14:05 JelleZijlstra

Diff from mypy_primer, showing the effect of this PR on open source code:

archinstall (https://github.com/archlinux/archinstall)
- ./archinstall/tui/curses_menu.py:710: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
- https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
- Please report a bug at https://github.com/python/mypy/issues
- version: 1.15.0
- ./archinstall/tui/curses_menu.py:710: : note: use --pdb to drop into pdb
- Traceback (most recent call last):
-   File "mypy/checkexpr.py", line 5903, in accept
-   File "mypy/nodes.py", line 1889, in accept
-   File "mypy/checkexpr.py", line 3284, in visit_member_expr
-   File "mypy/checkexpr.py", line 3309, in analyze_ordinary_member_access
-   File "mypy/checkmember.py", line 207, in analyze_member_access
-   File "mypy/checkmember.py", line 226, in _analyze_member_access
-   File "mypy/checkmember.py", line 344, in analyze_instance_member_access
-   File "mypy/meet.py", line 96, in meet_types
-   File "mypy/subtypes.py", line 223, in is_proper_subtype
-   File "mypy/subtypes.py", line 351, in _is_subtype
-   File "mypy/types.py", line 1469, in accept
-   File "mypy/subtypes.py", line 581, in visit_instance
-   File "mypy/subtypes.py", line 2095, in infer_class_variances
-   File "mypy/subtypes.py", line 2057, in infer_variance
-   File "mypy/subtypes.py", line 186, in is_subtype
-   File "mypy/subtypes.py", line 351, in _is_subtype
-   File "mypy/types.py", line 3033, in accept
-   File "mypy/subtypes.py", line 1079, in visit_partial_type
- RuntimeError: Partial type "<partial list[?]>" cannot be checked with "issubtype()"

github-actions[bot] avatar May 30 '25 23:05 github-actions[bot]

I'm becoming skeptical that the way we're using TypeVar defaults has been a good idea; it can lead to usability issues quite easily.

As the person who added that part to the xml stubs, I believe my thought process was that tag is str in the normal use cases, so I didn't want downstream consumers to have to worry about it unless they were using one of the weirder use cases.

This particular issue came up because it seems like I didn't look at the uses of Element without parameters that existed in the stubs before my MR, which was definitely an oversight.

tungol avatar Aug 20 '25 22:08 tungol

stubtest errors are unrelated.

srittau avatar Oct 08 '25 12:10 srittau

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Oct 08 '25 12:10 github-actions[bot]