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

Fixes #1429, add iri2uri #1902

Merged
merged 11 commits into from May 19, 2022
Merged

Fixes #1429, add iri2uri #1902

merged 11 commits into from May 19, 2022

Conversation

ghost
Copy link

@ghost ghost commented May 9, 2022

Fixes #1429

#1429 (comment))

why park this in the "restore-pyrdfa" branch? This fix will work for non-HTML sources with non-ASCII chars in the IRI, perhaps something like http://example.com/thing/Almería.ttl for instance. So this is a generic fix for parse()

Summary of changes

Add an iri-to-uri conversion utility to encode IRIs to URIs for Graph.parse() sources. Added a couple of tests because feeding it with a suite of IRIs to check seems overkill (not that I could find one).

Checklist

  • Checked that there aren't other open pull requests for the same change.
  • Added tests for any changes that have a runtime impact.
  • Checked that all tests and type checking passes.
  • For changes that have a potential impact on users of this project:
    • Updated relevant documentation to avoid inaccuracies.
    • Considered adding additional documentation.
    • Considered adding an example in ./examples for new features.
    • Considered updating our changelog (CHANGELOG.md).
  • Considered granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.

@coveralls
Copy link

coveralls commented May 9, 2022

Coverage Status

Coverage increased (+0.009%) to 88.431% when pulling 72f5b59 on gjhiggins:fix-for-issue1429 into ccb9c4a on RDFLib:master.

@ghost ghost self-requested a review May 9, 2022 21:55
@sonarcloud
Copy link

sonarcloud bot commented May 15, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

rdflib/parser.py Outdated Show resolved Hide resolved
@aucampia aucampia requested a review from a team May 17, 2022 15:37
@aucampia aucampia self-assigned this May 17, 2022
rdflib/parser.py Outdated Show resolved Hide resolved
- Move `rdflib.parser.iri2uri` to `rdflib.util._iri2uri`

  In part this is because iri2uri does not operate on all IRIs, just
  http and https IRIs, but furthermore it is because this is not
  something essential to working with RDF.

- Expanded tests for `iri2uri` to cover each part of the URI.

- Convert the test that connects to dbpedia to using httpmock.

  This is so we have more control over response and can explicitly
  verify the request path.

- Some changes to imports to reduce the probability of circular imports.
@aucampia
Copy link
Member

Made some changes now to your branch, in brief:

  • Move rdflib.parser.iri2uri to rdflib.util._iri2uri

    In part this is because iri2uri does not operate on all IRIs, just
    http and https IRIs, but furthermore it is because this is not
    something essential to working with RDF.

  • Expanded tests for iri2uri to cover each part of the URI.

  • Convert the test that connects to dbpedia to using httpmock.

    This is so we have more control over response and can explicitly
    verify the request path.

  • Some changes to imports to reduce the probability of circular imports.

Copy link
Member

@aucampia aucampia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aucampia aucampia requested a review from a team May 17, 2022 19:32
@aucampia aucampia added the review wanted This indicates that the PR is ready for review label May 17, 2022
@aucampia aucampia removed their assignment May 17, 2022
Copy link
Contributor

@edmondchuc edmondchuc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Looks good.

@aucampia aucampia merged commit 32923ce into RDFLib:master May 19, 2022
@ghost ghost removed the review wanted This indicates that the PR is ready for review label Jun 4, 2022
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.

Graph parse module does not handle URI / IRI with non-ascii characters
3 participants