-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
bpo-28598: Support __rmod__ for RHS subclasses of str in % string formatting operations #51
bpo-28598: Support __rmod__ for RHS subclasses of str in % string formatting operations #51
Conversation
…rations. When you use `'%s' % SubClassOfStr()`, where `SubClassOfStr.__rmod__` exists, the reverse operation is ignored as normally such string formatting operations use the `PyUnicode_Format()` fast path. This patch tests for subclasses of `str` first and picks the slow path in that case. Fixes issue28598.
Open question: Should this patch be backported to 3.6 and 3.5? It's a bug fix, not a new feature. |
Yes, it should be backported if it's a bug (I haven't investigated myself yet.)
We don't use |
Ta, also for the title change! Updated the 'fixes' line too. |
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.
In general LGTM except few minor details.
And I'm not sure about the style of commit messages now. Should it include issue number at the first line? Should it contain "Patch by YYY." (now the author is specified in metadata)?
Python/ceval.c
Outdated
PyNumber_Remainder(dividend, divisor); | ||
PyObject *res; | ||
if (PyUnicode_CheckExact(dividend) && !( | ||
PyUnicode_Check(divisor) && !PyUnicode_CheckExact(divisor))) { |
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.
Too many negations.
I would expand the expression as PyUnicode_CheckExact(dividend) && (!PyUnicode_Check(divisor) || PyUnicode_CheckExact(divisor))
.
Misc/NEWS
Outdated
@@ -10,6 +10,8 @@ What's New in Python 3.7.0 alpha 1? | |||
Core and Builtins | |||
----------------- | |||
|
|||
- Issue #28598: Support __rmod__ for subclasses of str being called before str.__mod__. |
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.
Add "Patch by Martijn Pieters."
Codecov Report
@@ Coverage Diff @@
## master #51 +/- ##
==========================================
- Coverage 82.37% 82.37% -0.01%
==========================================
Files 1427 1427
Lines 350948 350953 +5
==========================================
- Hits 289099 289097 -2
- Misses 61849 61856 +7 Continue to review full report at Codecov.
|
Should this really be backported? It seems like a change in semantics more than a bugfix. |
@brettcannon This is definitely a bug, not a change in semantics. From the
|
Allow miss-islington to merge the PR if all of these conditions met: - All CI passed - At least one core dev approved - PR is made by miss-islington
Back port latest changes to importlib.resources Closes python#51 See merge request python-devs/importlib_resources!55
…ython#51) * feat: store bb_test flag in frame to reload during resume * feat: bigger CACHE entries for everything, CACHE for JUMP_BACKWARD * nit: increase overallocate factor even more * feat: remove bb_flag/bb_test and use only the frame's
When you use
'%s' % SubClassOfStr()
, whereSubClassOfStr.__rmod__
exists, the reverse operation is ignored as normally such string formatting operations use thePyUnicode_Format()
fast path. This patch tests for subclasses ofstr
first and picks the slow path in that case.Fixes bpo-28598.