Comments are not accounted for

[eemeli]

At least the following make using the types somewhat challenging:

• The tag of an element can be etree.Comment, a function.
• The text of a comment is always a str.

Given these, I wouldn't be surprised to find other places where the types are misaligned with respect to comments.

[abelcheung]

So I got it all wrong.

@eemeli I presume you mean the tag selector here, please tell me (or even better, provide an example) if I didn't get your real intention.

It is indeed a challenge on how to fit in other types of element factory into the _TagSelector alias, but ultimately there's no good solution. Not just etree.Comment(), there are etree.Entity() and etree.ProcessingInstruction() too, all of them having different call arguments.

That's why I choose to use generic Callable in _TagSelector, this should cover usage of etree.Comment too. For example, the following code should be ok for type checkers:

    from lxml import etree
    tree = etree.parse(somefile)
    result = tree.iter(etree.Comment)

[eemeli]

The code that I encountered these issues with looks roughly like this:

from lxml import etree
root = etree.fromstring('<foo><!-- comment --></foo>')
comment: list[str | None] = []  # TODO: should be list[str]
for el in root:
    if el.tag == etree.Comment:  # type: ignore[comparison-overlap]
        comment.append(el.text)

To satisfy mypy, I need the type: ignore comment because the current types are presuming that el.tag is always a string, and it isn't for comments.

Also, even if I narrow the typing of el with cast(etree._Comment, el), el.text won't be typed as str, but always as str | None; hence the wider typing for comment.

[abelcheung]

For the first problem, I don't have any good solution so far. This is the price needed to pay when I decided to follow lxml on how _Comment is inherited from _Element. _Comment has totally different behavior from _Element, yet it is a subclass of _Element. I could have severed the relation between _Comment and _Element, but that leads to another set of problems.

Right now you may want to try narrowing down the type of el itself, for example:

for el in root:
    if isinstance(el, etree._Comment):
        comment.append(el.text)

On second problem, you're right. It is possible to use None as input argument in etree.Comment(), but the .text attribute of comment element is always str. Will fix that soon. Done.

[eemeli]

The instanceof check is indeed better, and the text fix looks promising. Thank you!

[abelcheung]

The fixable part of this issue is addressed, so I'm closing. Please feel free to reopen if something related to this issue still doesn't work.