-
-
Couldn't load subscription status.
- Fork 33.2k
bpo-46752: Uniform TaskGroup.__repr__ #31409
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
Conversation
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.
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.)
Lib/asyncio/taskgroups.py
Outdated
| info_str = ' ' + ' '.join(info) | ||
| return f'<TaskGroup{info_str}>' |
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.
| info_str = ' ' + ' '.join(info) | |
| return f'<TaskGroup{info_str}>' | |
| info_str = ' '.join(info) | |
| return f'<TaskGroup {info_str}>' |
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.
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?
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.
IMO accumulating strings in [] isn't a great pattern unless you have to use it for performance reasons. /my2c.
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.
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?
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.
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).
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.
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.
|
Oops, I've mentioned the problem ': vs =' problem but didn't actually resolve it. |
|
@gvanrossum do you want tests for |
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.
No need for tests. Maybe a new contributor will add them. :-)
Other asyncio objects use
prop=valformat, notprop:val.https://bugs.python.org/issue46752