Logging exceptions from Literal value converters

[mwatts15]

Summary of changes

Adds logging at warning level for exceptions raised when trying to convert a Literal's lexical form into Python-value space. Also refactored to not have convFunc be a boolean -- I think it should resolve the type-checking issue , but I don't know how to run the type checker.

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.

[aucampia]

Some options for running type checking:

With docker compose:

docker compose build
docker compose run --rm devcontainer mypy --show-error-context --show-error-codes

Or:

docker compose build
docker compose run --rm devcontainer task validate:static

With tox - this will run mypy and unit tests after:

tox -e py37

With go-task in a virtual environment:

task WITH_VENV=1 venv:install validate:static

[mwatts15]

@aucampia what's the policy on updating function/variable names per the "extra-tasks" job? They're pre-existing names, so I didn't see fit to update them.

[coveralls]

Coverage increased (+0.006%) to 88.437% when pulling bb152a7 on mwatts15:master into ba855c2 on RDFLib:master.

[aucampia]

@aucampia what's the policy on updating function/variable names per the "extra-tasks" job? They're pre-existing names, so I didn't see fit to update them.

It is okay if you don't for now, flakeheaven is the best option we have for gradually getting the codebase to pass with flake8, it is not perfect though, as the baseline kind of gets confused when a file is edited, so sometimes you get errors like you saw now. I will fix them after merging, the most important thing is that we want to catch new code that causes flake8 errors.

That is also why I made it report errors separately.

[mwatts15]

win 3.10 failure is an unrelated socket error. could someone re-run?

[aucampia]

win 3.10 failure is an unrelated socket error. could someone re-run?

Will do, just waiting for others to complete.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[aucampia]

Your change looks good, I'm a bit on the fence of using logging.warning though, it has the benefit over warnings.warn that you can see the stack trace, but it has the downside that it is less easy for users to handle programmatically.

The rest of the change makes some much needed improvement.

On a related note, you may be interested in the recently added https://rdflib.readthedocs.io/en/latest/apidocs/rdflib.html#rdflib.term.Literal.ill_formed

Which is basically what this warning will be for:

https://www.w3.org/TR/rdf11-concepts/#section-Graph-Literal

Otherwise, the literal is ill-typed and no literal value can be associated with the literal. Such a case produces a semantic inconsistency but is not syntactically ill-formed. Implementations must accept ill-typed literals and produce RDF graphs from them. Implementations may produce warnings when encountering ill-typed literals.

[mwatts15]

@aucampia thanks. I hit this because I didn't have html5lib installed and parsing for the HTML literal failed (probably should have mentioned in the description...)

Regarding warnings vs logging, main thing I wanted was a stack trace. I tend to see logs from other modules even when I don't see warnings -- not exactly conclusive, but it's pretty consistent for the applications I've worked on.

[aucampia]

Regarding warnings vs logging, main thing I wanted was a stack trace. I tend to see logs from other modules even when I don't see warnings -- not exactly conclusive, but it's pretty consistent for the applications I've worked on.

I think this is reasonable, without a stack trace the warnings are of limited use, and while getting stack traces from warnings.warn is technically possible, it certainly is not convenient.