feat: make lxml optional (but don't yet remove from requirements) #166
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tlambert03
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes slight modifications to the
elem2dictfunction that makes it compatible with both lxml as well as standard lib xml.etree elements. I think the vast majority of the speedup of lxml compared to xmlschema parsing is the lack of schema validation. (i saw ~25% speedup with lxml over xml.etree ... nice, but shouldn't be mandatory given that lxml can lag behind with updates)My hope here, in the next couple PRs is to disconnect the validation of incoming xml from the parsing of xml. We could make both xmlschema and lxml optional (needed only for validation of an xml instance against the schema ... which we don't particularly need here, given that pydantic will do that on the dict object)