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

add lxml typechecking #279

Merged
merged 3 commits into from
Jun 20, 2024
Merged

Conversation

jkhsjdhjs
Copy link
Contributor

lxml provides type annotations via the lxml-stubs package. By using this package, we don't need the type: ignore comments when importing lxml anymore. Furthermore, the typehints are adjusted: etree.Element is only a factory function that creates etree._Element instances.

Finally, when generating the docs we introduced a workaround that makes linking to the lxml documentation possible. Since it only replaced lookups for Element with _Element, it isn't necessary anymore and removed.

Now that we require `lxml-stubs`, we can remove the `type: ignore`
coments from the `lxml` imports. To make mypy happy, many typehints are
adjusted, mostly `etree.Element` -> `etree._Element`.
Now that we use `etree._Element` instead of `etree.Element` in our
docstrings and type annotations, we don't need this workaround anymore.
@s-heppner
Copy link
Contributor

So you're saying that by default, every user of lxml should access the internal class etree._Element?
That seems weird to me.

@jkhsjdhjs
Copy link
Contributor Author

Every user of lxml is implicitly working with _Element, as Element is just a factory function that returns _Element instances. It's also a bit weird to me, but that's the way it is implemented.

See also: https://stackoverflow.com/questions/76321801/type-hints-for-lxml-etree-element#comment134587997_76321801

@s-heppner
Copy link
Contributor

Okay, then I'm fine that we use etree._Element for the type annotation, as long as we still call etree.Element() to instantiate.

@s-heppner s-heppner merged commit ce1568f into eclipse-basyx:main Jun 20, 2024
8 checks passed
@s-heppner s-heppner deleted the add/lxml_typing branch June 20, 2024 11:22
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

Successfully merging this pull request may close these issues.

2 participants