Skip to content

fix: handling of Literal datatype #2076

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

Merged
merged 2 commits into from
Aug 12, 2022

Conversation

aucampia
Copy link
Member

@aucampia aucampia commented Aug 7, 2022

Summary of changes

Check datatype against None instead of checking it's truthiness (i.e.
if datatype is not None: instead of if datatype:).

Checking truthiness instead of is not None causes a blank string to
be treated the same as None. The consequence of this was that
Literal.datatype could be a str, a URIRef or None, instead of
just a URIRef or None as was seemingly intended.

Other changes:

  • Changed the type of Literal.datatype to be Optional[URIRef]
    instead of Optional[str] now that str will always be converted to
    URIRef even if it is a blank string.
  • Changed rdflib.util._coalesce to make it easier and safer to use
    with a non-None default value.
  • Added rdflib.util._convert_optional that makes it easy to convert
    optional values while retaining their optional nature.
  • Changed rdflib.util to avoid issues with circular imports.

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:
    • 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.

@aucampia aucampia force-pushed the iwana-20220807T1740-datatype_uri branch 2 times, most recently from fb8e1f7 to 476bce1 Compare August 7, 2022 20:57
@coveralls
Copy link

coveralls commented Aug 7, 2022

Coverage Status

Coverage increased (+0.003%) to 90.451% when pulling f31625e on aucampia:iwana-20220807T1740-datatype_uri into 131d9e6 on RDFLib:master.

@aucampia aucampia force-pushed the iwana-20220807T1740-datatype_uri branch from 476bce1 to ab23328 Compare August 7, 2022 21:40
@aucampia aucampia marked this pull request as ready for review August 7, 2022 21:42
@aucampia aucampia force-pushed the iwana-20220807T1740-datatype_uri branch from ab23328 to ee0a4e9 Compare August 7, 2022 21:43
Check datatype against `None` instead of checking it's truthiness (i.e.
`if datatype is not None:` instead of `if datatype:`).

Checking truthiness instead of `is not None` causes a blank string to
be treated the same as None. The consequence of this was that
`Literal.datatype` could be a `str`, a `URIRef` or `None`, instead of
just a `URIRef` or `None` as was seemingly intended.

Other changes:
- Changed the type of `Literal.datatype` to be `Optional[URIRef]`
  instead of `Optional[str]` now that `str` will always be converted to
  `URIRef` even if it is a blank string.
- Changed `rdflib.util._coalesce` to make it easier and safer to use
  with a non-`None` default value.
- Added `rdflib.util._convert_optional` that makes it easy to convert
  optional values while retaining their optional nature.
- Changed `rdflib.util` to avoid issues with circular imports.
@aucampia aucampia force-pushed the iwana-20220807T1740-datatype_uri branch from ee0a4e9 to 713af0d Compare August 7, 2022 21:47
@aucampia aucampia requested a review from a team August 7, 2022 21:55
@aucampia aucampia added review wanted This indicates that the PR is ready for review and removed review wanted This indicates that the PR is ready for review labels Aug 10, 2022
@aucampia aucampia marked this pull request as draft August 10, 2022 00:50
@aucampia aucampia removed the request for review from a team August 10, 2022 00:51
Easy enough to just use `z = None if None else convert(x)`
@aucampia aucampia marked this pull request as ready for review August 10, 2022 18:30
@aucampia aucampia requested a review from a team August 10, 2022 18:30
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

👍

@aucampia aucampia merged commit a39d143 into RDFLib:master Aug 12, 2022
@aucampia aucampia deleted the iwana-20220807T1740-datatype_uri branch April 9, 2023 15:05
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