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

Issue 2378: Remove unicode string form in rdflib/term.py #2384

Merged
merged 2 commits into from May 19, 2023

Conversation

ajnelson-nist
Copy link
Contributor

Summary of changes

This PR updates some docstrings to no longer use the Python 2 unicode string pattern (u"some string").

This PR is scoped to the original file where I'd encountered the problem, running python3 -m doctest rdflib/term.py.

One outstanding doctest issue remains, but rather than attempt to fix everything, this PR is filed now to make incremental progress, and trial the goal progression laid out in #2383.

The last issue for rdflib/term.py, which I think someone else will need to file a PR for, is evidenced in this shell transcript:

$ python3 -m doctest rdflib/term.py
**********************************************************************
File "/private/tmp/rdflib/rdflib/term.py", line 677, in term.Literal
Failed example:
    Literal(1) > URIRef('foo') # by node-type
Exception raised:
    Traceback (most recent call last):
      File "/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/doctest.py", line 1336, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest term.Literal[19]>", line 1, in <module>
        Literal(1) > URIRef('foo') # by node-type
    TypeError: '>' not supported between instances of 'Literal' and 'URIRef'
Failed to convert Literal lexical form to value. Datatype=http://www.w3.org/2001/XMLSchema#integer, Converter=<class 'int'>
Traceback (most recent call last):
  File "/private/tmp/rdflib/rdflib/term.py", line 2084, in _castLexicalToPython
    return conv_func(lexical)  # type: ignore[arg-type]
ValueError: invalid literal for int() with base 10: 'a'
**********************************************************************
1 items had failures:
   1 of  21 in term.Literal
***Test Failed*** 1 failures.

Checklist

  • Checked that there aren't other open pull requests for
    the same change.
  • Added tests for any changes that have a runtime impact. (N/A)
  • Checked that all tests and type checking passes. (Pending CI results, all tests pass in main, and more tests pass in the "Goal" branch)
  • For changes that have a potential impact on users of this project:
    • Updated relevant documentation to avoid inaccuracies. (N/A)
    • Considered adding additional documentation. (N/A)
    • Considered adding an example in ./examples for new features. (N/A)
    • 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.

References:
* RDFLib#2378

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
@ajnelson-nist
Copy link
Contributor Author

I'm not sure why something went awry for only Python 3.11. After I worked through documenting #2385, I was able to get this log output:

================================================== FAILURES ===================================================
______________________________________ test_guess_format_for_parse[None] ______________________________________
Traceback (most recent call last):
  File "/private/tmp/rdflib/rdflib/plugin.py", line 137, in get
    p: Plugin[PluginT] = _plugins[(name, kind)]
                         ~~~~~~~~^^^^^^^^^^^^^^
KeyError: ('text/plain', <class 'rdflib.parser.Parser'>)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/private/tmp/rdflib/test/test_graph/test_graph.py", line 347, in test_guess_format_for_parse
    graph.parse(location="http://www.w3.org/ns/adms.ttl")
  File "/private/tmp/rdflib/rdflib/graph.py", line 1489, in parse
    parser = plugin.get(format, Parser)()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/private/tmp/rdflib/rdflib/plugin.py", line 139, in get
    raise PluginException("No plugin registered for (%s, %s)" % (name, kind))
rdflib.plugin.PluginException: No plugin registered for (text/plain, <class 'rdflib.parser.Parser'>)
-------------------------------------------- Captured stderr call ---------------------------------------------
127.0.0.1 - - [12/May/2023 11:36:40] "GET /file/4b5b7d9e0d9c4cbaafda954b640dd450 HTTP/1.1" 200 -

It seems this is unrelated enough to this PR that it's not a blocking issue, but this might be a heads-up that CI is going to complain the next time someone pushes to main.

@ghost
Copy link

ghost commented May 12, 2023

I'm not sure why something went awry for only Python 3.11.
...
It seems this is unrelated enough to this PR that it's not a blocking issue, but this might be a heads-up that CI is going to complain the next time someone pushes to main.

fwiw I can confirm it's unrelated to this PR, aucampia is already working to resolve the issue: #2382

@coveralls
Copy link

Coverage Status

Coverage: 90.85%. Remained the same when pulling 55f4906 on ajnelson-nist:issue-2378-rdflib-term-ustrings into fec7f0a on RDFLib:main.

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.

Thanks for the PR, looks good to me.

@aucampia aucampia requested a review from a team May 19, 2023 12:52
@aucampia aucampia added documentation Improvements or additions to documentation review wanted This indicates that the PR is ready for review ready to merge The PR will be merged soon if no further feedback is provided. labels May 19, 2023
@aucampia aucampia merged commit ddcc4eb into RDFLib:main May 19, 2023
25 checks passed
@ajnelson-nist ajnelson-nist deleted the issue-2378-rdflib-term-ustrings branch May 22, 2023 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation ready to merge The PR will be merged soon if no further feedback is provided. review wanted This indicates that the PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants