Skip to content
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

_Attrib type is not comparable to dict[str, str] even though it works at runtime #98

Open
DetachHead opened this issue Jan 12, 2024 · 4 comments

Comments

@DetachHead
Copy link

from lxml.etree import XML

assert XML(b"<a foo='bar'></a>").attrib == {'foo': 'bar'}

mypy complains about this code, even though it works at runtime:

Non-overlapping equality check (left operand type: "_Attrib", right operand type: "dict[str, str]")
@scoder
Copy link
Member

scoder commented Jan 13, 2024

It's sometimes difficult to completely map lxml's pythonic API to static type annotations. PR welcome.

@DetachHead
Copy link
Author

i think the fix is to add a __eq__ to _Atrib, but that's not ideal because that would incorrectly make it comparable to anything, since object.__eq__ takes object that means any subtypes have to do the same:

class object:
    def __eq__(self, __value: object) -> bool: ...

class _Attrib:
    def __eq__(self, other: _Attrib | _DictAnyStr) -> bool: ... # error: Argument 1 of "__eq__" is incompatible with supertype "object"; supertype defines the argument type as "object"

(i proposed a solution in KotlinIsland/basedmypy#574 (comment) but i'm sure that would break lots of existing code)

@scoder
Copy link
Member

scoder commented Jan 15, 2024

Well, it obviously has an __eq__ method, otherwise, it wouldn't compare equal to a dict. And the current implementation really just says "comparable to anything that dict() accepts". Whatever that comprises in a given Python version.

I don't really see why we should be specific about the things that _Attrib can be compared with. If users want to compare it to some arbitrary other object, it seems more reasonable to allow than deny that. Not all constraints can or should be expressed in terms of types. This is Python, after all.

@Dreamsorcerer
Copy link

Not all constraints can or should be expressed in terms of types.

On the contrary, this is perfectly covered by the typing system. If _Attrib is not comparable with any arbitrary object, then that would be a bug in lxml, generally any Python object should be comparable to any other object.

If we want to create a type error for the user, then we could use overloads to be a bit smarter about it:

@overload
def __eq__(self, other: Mapping[str, str]) -> bool:
    ...
@overload
def __eq__(self, other: object) -> Literal[False]:
    ...

In this case, if the user compares with anything that is not a compatible mapping, then a type checker like mypy can produce an error saying that the comparison is redundant, as it will always return False.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants