Skip to content

Conversation

furkanonder
Copy link
Contributor

@furkanonder furkanonder commented May 17, 2023

@sunmy2019 sunmy2019 added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label May 22, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @sunmy2019 for commit bc0270d 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label May 22, 2023
@sunmy2019

This comment was marked as outdated.

@sunmy2019
Copy link
Member

Target to 3.13

Copy link
Member

@sunmy2019 sunmy2019 left a comment

Choose a reason for hiding this comment

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

I think there are some problems here.

  1. We cannot utilize the PyBytes_Repr from Python side since bytes.__repr__ will do checks.
>>> bytes.__repr__(bytearray([1]))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: descriptor '__repr__' requires a 'bytes' object but received a 'bytearray'
  1. Although PyBytes_Repr is a public API, applying it for not bytes (but supporting buffer protocol) is still weird. As it clearly states, it's a repr of bytes.

  2. Using the PyObject_GetBuffer for bytes slightly hurts the performance of bytes objects.

I think 1 and 2 is the main problem here. I would propose a new function that starts with PyBuffer_*, maybe PyBuffer_Repr.

Then, move the implementation to some function foo.

Then, call this foo from both PyBuffer_Repr and PyBytes_Repr.

sunmy2019

This comment was marked as duplicate.

@sunmy2019
Copy link
Member

@serhiy-storchaka What do you think?

@furkanonder furkanonder added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Aug 6, 2023
@furkanonder furkanonder added the type-feature A feature request or enhancement label Aug 19, 2025
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

This PR contains only the part of the original patch. It does not change the repr of bytearray.

And it shares the flaw of the original patch -- it does not work with nested classes and classes with non-ASCII names.

See #138181 that solves this problem in other way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants