-
Notifications
You must be signed in to change notification settings - Fork 565
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
Convert old string substitutions to f-strings in term.py #2864
Conversation
…mance when stringifying URIRef, BNode, and Literal nodes, also replace other outdated patterns in term.py with some modern equivelents for better performance and compatibility.
Additionally, one private internal function is removed from term.py, that is Removing this speeds up creation of BNodes (especially when many BNodes need to be created) because it removes one layer of indirection and one additional python method call. The ability to pass your own generator function if you need a different |
Notice there is an odd behaviour in the tests. The previous version had the assertion that Literal Substitution of |
…dn't work as a BNode prefix generator, even though it should have. This was revieled by a newly failing test after the change to better string concatenation in the BNode constructor.
One last change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one. Speed improvements, bug fixes and readability!
I thought this work had already been done around 18 months ago.
Or maybe there was and old PR that does it? I'm not sure.
As-of Python 3.8, f-strings are feature-comparable with
str.format()
, and have even eventually surpassed the raw performance of old-style "%-substitution" strings from the Python2 days.This PR does the follow:
% substitution
strings interm.py
Some micro-benchmarking to show the kind of differences:
results