← 返回首页
gh-98836: Extend PyUnicode_FromFormat() by serhiy-storchaka · Pull Request #98838 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted
Cancel Submit feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel Create saved search
Appearance settings
Resetting focus

gh-98836: Extend PyUnicode_FromFormat()#98838

Merged
serhiy-storchaka merged 15 commits into
python:mainfrom
serhiy-storchaka:capi-unicode-fromformat
May 21, 2023
Merged

gh-98836: Extend PyUnicode_FromFormat()#98838
serhiy-storchaka merged 15 commits into
python:mainfrom
serhiy-storchaka:capi-unicode-fromformat

Conversation

serhiy-storchaka commented Oct 29, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member
  • Support for conversion specifiers o (octal) and X (uppercase hexadecimal).
  • Support for length modifiers j (intmax_t) and t (ptrdiff_t).
  • Length modifiers are now applied to all integer conversions.
  • Support for wchar_t C strings (%ls and %lV).
  • Support for variable width and precision (*).
  • Support for flag - (left alignment).

pythongh-98836: Extend PyUnicode_FromFormat() …
aca2393
* Support for conversion specifiers o (octal) and X (uppercase hexadecimal). * Support for length modifiers j (intmax_t) and t (ptrdiff_t). * Length modifiers are now applied to all integer conversions. * Support for wchar_t C strings (%ls and %lV). * Support for variable width and precision (*). * Support for flag - (left alignment).
bedevere-bot mentioned this pull request Nov 3, 2022
Closed
serhiy-storchaka marked this pull request as ready for review November 3, 2022 15:23
serhiy-storchaka requested review from vstinner and removed request for lysnikolaou and pablogsal November 3, 2022 18:02
vstinner reviewed Nov 4, 2022
View reviewed changes
Comment thread Doc/whatsnew/3.12.rst Outdated
:c:func:`PyUnicode_FromFormatV` now sets a :exc:`SystemError`.
In previous versions it caused all the rest of the format string to be
copied as-is to the result string, and any extra arguments discarded.
(Contributed by Serhiy Storchaka in :gh:`95781`.)

vstinner Nov 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

It seems like this paragraph was duplicated by mistake?

serhiy-storchaka Nov 6, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

True.

Comment thread Lib/test/test_unicode.py Outdated
c_int, c_long, c_longlong, c_ssize_t,
c_uint, c_ulong, c_ulonglong, c_size_t, c_void_p)
c_uint, c_ulong, c_ulonglong, c_size_t, c_void_p,
sizeof, c_wchar, c_wchar_p)

vstinner Nov 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Can you please a comment somewhere to list formats which are not tested and the reason why?

I see at least %j and %t which are not tested. Please mention that _testcapi.test_string_from_format() has a wider coverage of all formats, and test these formats.

serhiy-storchaka Nov 6, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Done.

Comment thread Lib/test/test_unicode.py Outdated
check_format(' 0000123', b'%10.7i', c_int(123))
check_format('0000000123', b'%010.7i', c_int(123))
check_format('0000123 ', b'%-10.7i', c_int(123))
check_format('0000123 ', b'%-010.7i', c_int(123))

vstinner Nov 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Would it be possible to generate these tests? The code is similar for the 6 groups of tests.

serhiy-storchaka Nov 6, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

It is not so similar. It is different for signed and unsigned types, but I'll try to generalize it.

vstinner Nov 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

I just suggested to generate these tests. It's ok to leave them as they are if it's too complicated to generate them.

Comment thread Lib/test/test_unicode.py Outdated
b'%V', None, b'xyz')

# test %ls
check_format('abc', b'%ls', c_wchar_p('abc'))

vstinner Nov 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

You can please add one non-ASCII character in wchar tests, %ls and %lV? In addition to tests truncating to 2 wchar_t.

serhiy-storchaka Nov 6, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

What do you mean? Isn't the following line enough?

vstinner Nov 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

I'm thinking at adding a test with non-ASCII characters which fit into UCS-2 (16-bit wchar_t) characters. Something like:

check_format('a\xe9\u20ac', b'%ls', c_wchar_p('a\xe9\u20ac'))

Some bugs are only triggered depending on the maximum code point, and wchar_t code can take different code path depending if there is a surrogate character or not.

serhiy-storchaka Nov 20, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Done.

Comment thread Lib/test/test_unicode.py Outdated
@@ -2880,10 +2989,11 @@ def check_format(expected, format, *args):
# check for crashes

vstinner Nov 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

this comment is misleading. Python doesn't crash but reject invalid format string with a SyntaxError. Would you mind to rephrase the comment? Like: # test invalid format strings?

serhiy-storchaka Nov 6, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Done. Restored the part of the older comment.

Comment thread Doc/c-api/unicode.rst
| | | :c:func:`PyObject_Repr`. |
+-------------------+---------------------+----------------------------------+
ASCII-encoded string.

vstinner Nov 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Can you please add something like [[fill]align][sign][z][#][0][width][grouping_option][.precision][type]? Example taken from: https://docs.python.org/dev/library/string.html#format-specification-mini-language

The printf format is complex, and it's not obvious in which order each part should be written. I understand that it's something like:

%[conversion flags][minimum width][.precision][length modifier][conversion type]

Or if you prefer a shorter version:

%[conversion][width][.precision][modifier][conversion]

serhiy-storchaka Nov 6, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

I copied the following paragraphs from the description of the print-like formatting in Python: https://docs.python.org/3/library/stdtypes.html#printf-style-string-formatting.

I think that it also can benefit from from similar changes, but let leave it to other issue.

vstinner approved these changes Nov 7, 2022
View reviewed changes

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

LGTM.

Comment thread Lib/test/test_unicode.py Outdated
check_format(' 0000123', b'%10.7i', c_int(123))
check_format('0000000123', b'%010.7i', c_int(123))
check_format('0000123 ', b'%-10.7i', c_int(123))
check_format('0000123 ', b'%-010.7i', c_int(123))

vstinner Nov 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

I just suggested to generate these tests. It's ok to leave them as they are if it's too complicated to generate them.

Comment thread Lib/test/test_unicode.py Outdated
# Length modifiers "j" and "t" are not tested here because ctypes does
# not expose types for intmax_t and ptrdiff_t.
# _testcapi.test_string_from_format() has a wider coverage of all
# formats.

vstinner Nov 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Thank you, this comment is useful to me at least :-)

Comment thread Lib/test/test_unicode.py Outdated
b'%V', None, b'xyz')

# test %ls
check_format('abc', b'%ls', c_wchar_p('abc'))

vstinner Nov 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

I'm thinking at adding a test with non-ASCII characters which fit into UCS-2 (16-bit wchar_t) characters. Something like:

check_format('a\xe9\u20ac', b'%ls', c_wchar_p('a\xe9\u20ac'))

Some bugs are only triggered depending on the maximum code point, and wchar_t code can take different code path depending if there is a surrogate character or not.

vstinner commented Nov 7, 2022

Copy link
Copy Markdown
Member

Feel free to ignore my suggestions, the PR LGTM.

vstinner approved these changes Nov 21, 2022
View reviewed changes

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

LGTM.

serhiy-storchaka merged commit f3466bc into python:main May 21, 2023
serhiy-storchaka deleted the capi-unicode-fromformat branch May 21, 2023 21:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

vstinner vstinner approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Footer

© 2026 GitHub, Inc.