bpo-46752: Uniform TaskGroup.__repr__ - #31409
Conversation
Sorry, something went wrong.
gvanrossum
left a comment
There was a problem hiding this comment.
Didn't you say other asyncio objects use prop=val? Were you going to add that? (I guess it'll require updating a few tests.)
Sorry, something went wrong.
| info_str = ' ' + ' '.join(info) | ||
| return f'<TaskGroup{info_str}>' |
There was a problem hiding this comment.
| info_str = ' ' + ' '.join(info) | |
| return f'<TaskGroup{info_str}>' | |
| info_str = ' '.join(info) | |
| return f'<TaskGroup {info_str}>' |
Sorry, something went wrong.
There was a problem hiding this comment.
After applying the suggestion, a repr for created TaskGroup before __enter__() is called will be "<TaskGroup >" (note the space after class name).
Is it ok? Should we care about this rare case?
Sorry, something went wrong.
There was a problem hiding this comment.
IMO accumulating strings in [] isn't a great pattern unless you have to use it for performance reasons. /my2c.
Sorry, something went wrong.
There was a problem hiding this comment.
After applying the suggestion, a repr for created TaskGroup before __enter__() is called will be "<TaskGroup >" (note the space after class name). Is it ok? Should we care about this rare case?
The original code has the same issue AFAICT. If you don't want that you could initialize info = [''] perhaps?
Sorry, something went wrong.
There was a problem hiding this comment.
IMO accumulating strings in [] isn't a great pattern unless you have to use it for performance reasons. /my2c.
Meh. IIRC the info pattern is used all over asyncio (e.g. _task_repr_info in base_tasks.py).
Sorry, something went wrong.
There was a problem hiding this comment.
info = [''] is clever, applied.
Yes, info list is used by other asyncio repr's. I'd like to have the similar approach across the library.
Sorry, something went wrong.
|
Oops, I've mentioned the problem ': vs =' problem but didn't actually resolve it. |
Sorry, something went wrong.
|
@gvanrossum do you want tests for __repr__? |
Sorry, something went wrong.
gvanrossum
left a comment
There was a problem hiding this comment.
No need for tests. Maybe a new contributor will add them. :-)
Sorry, something went wrong.
Other asyncio objects use prop=val format, not prop:val.
https://bugs.python.org/issue46752