A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ericvsmith Thanks a lot! I've addressed almost all your comments, except ones that you clearly wanted to have some extra research on your side. Let me know if you need my help!

P.S. To make @bedevere-bot happy: I have made the requested changes; please review again

Thanks for making the requested changes!

@ericvsmith: please review the changes made to this pull request.

Thanks! I'll see if I can figure out why it's printing out more than just the format spec.

I'm also going to mark this as back-portable to 3.10 and 3.9, since changing exception text is okay in a dot release.

Here's what I don't understand:

>>> "{a:%k}".format(a='a')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Invalid format specifier '{a:%k}' for object of type 'str'
>>> format('a', '%k')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Invalid format specifier '%k' for object of type 'str'
>>> f'{"a":%k}'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Invalid format specifier '%k' for object of type 'str'

Why does the .format() error message say '{a:%k}', but the other two day %k?

For reference, this is what it looks like in 3.8 (the newest version I have around, but I don't think it makes a difference):

>>> "{a:%k}".format(a='a')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Invalid format specifier
>>> format('a', '%k')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Invalid format specifier
>>> f'{"a":%k}'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Invalid format specifier

In the meantime I will add more tests for format() function, % and f''

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ericvsmith Thanks! I've checked all other usages of PyUnicode_FromKindAndData, all other cases has explici NULL compare. So, I've modified it.

To make bot happy: I have made the requested changes; please review again.

Thanks for making the requested changes!

@ericvsmith: please review the changes made to this pull request.

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

I'm also going to mark this as back-portable to 3.10 and 3.9, since changing exception text is okay in a dot release.

I'm going to ask on python-dev and see how people feel about this.

I'm just waiting on the backport decision to hear if anyone comments on https://mail.python.org/archives/list/python-dev@python.org/message/G6H3M7USSQ2YCFMATMU24J5BJMDJPDVW/

The feedback on python-dev is to not backport this. Which is a bummer, but it's the right decision.

[Edit: there already is a blurb. Nevermind.]

@ericvsmith No worries!

could you create a what's new (blurb) entry for it?

You mean something like this? https://github.com/sobolevn/cpython/blob/4a11a03e8922af636c3ff0b3d6d7d6e255304c2b/Misc/NEWS.d/next/Library/2021-09-13-14-59-01.[bpo-20524](https://bugs.python.org/issue20524).PMQ1Fh.rst If yes, it is already included. If not, then I need more help - I am not quite familiar with the process - where should I put it? 🙂

@sobolevn: After I wrote that message, I realized you already had a well written blurb that I skipped over.

Thanks for all of your work on this issue. I think it's a great improvement. I just looked at the issue: 7.5 years in the making. Time flies!

Thanks a lot for your help, @ericvsmith! It was a pleasure 🤝

P.S. I have some other unreviewed PRs in case you are interested https://github.com/python/cpython/pulls/sobolevn?q=is:open 🙂