fix: SPARQL XML result parsing #2044
Merged
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
Fixed the following problems with the SPARQL XML result parsing: - Both the parse method of both the lxml and `xml` modules does not work work well with `TextIO` objects, the `xml` module works with `TextIO` objects if the XML encoding is `utf-8`, but not if it is `utf-16`, and with `lxml` parse fails for both `utf-8` and `utf-16`. To fix this I changed the XML result parser to first convert `TextIO` to `bytes` and then feed the `bytes` to `parse()` using `BytesIO`. - The parser was operating on all elements inside `results` and `result` elements, even if those elements were not `result` and `binding` elements respectively. This was causing problems with `lxml`, as `lxml` also returns comments when iterating over elements. To fix this I added a check for the element tags so that only the correct elements are considered. Other changes: - Added type hints to `rdflib.plugins.sparql.results.xmlresults`. - Run with `lxml` one some permutations in the test matrix. - Removed `rdflib.compat.etree`, as this was not very helpful for the SPARQL XML Result parser and it was not used elsewhere. - Added an `lxml` environment to tox which installs `lxml` and `lxml-stubs`.
aucampia
force-pushed
the
iwana-20220718T0603-lxml-test
branch
from
c106828
to
f011df1
Compare
|
@mielvds tagging you if you have time for a review. |
|
Just a heads up, I'm considering this somewhat of a last ditch effort to save lxml integration. Seeing as lxml integration was broken for a long time I don't think there is anyone relying on it, and if it is causing more problems and nobody is interested in maintaining it I will remove it. |
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.
Summary of changes
Fixed the following problems with the SPARQL XML result parsing:
xmlmodules does not workwork well with
TextIOobjects, thexmlmodule works withTextIOobjects if the XML encoding is
utf-8, but not if it isutf-16, andwith
lxmlparse fails for bothutf-8andutf-16. To fix this Ichanged the XML result parser to first convert
TextIOtobytesandthen feed the
bytestoparse()usingBytesIO.resultsandresultelements, even if those elements were notresultandbindingelements respectively. This was causing problems with
lxml, aslxmlalso returns comments when iterating over elements. To fix this I added
a check for the element tags so that only the correct elements are
considered.
Other changes:
rdflib.plugins.sparql.results.xmlresults.lxmlone some permutations in the test matrix.rdflib.compat.etree, as this was not very helpful for theSPARQL XML Result parser and it was not used elsewhere.
lxmlenvironment to tox which installslxmlandlxml-stubs.parameters.
Related issues:
Checklist
the same change.
CHANGELOG.md).so maintainers can fix minor issues and keep your PR up to date.