[core] Better Actor Representation #2369
Conversation
|
Test FAILed. |
python/ray/actor.py
Outdated
| return "Actor({})".format(self._ray_actor_id) | ||
|
|
||
| def __str__(self): | ||
| return "RayActor({})".format(self._ray_class_name) |
There was a problem hiding this comment.
Why put different information in these two functions? I think they should be the same.
There was a problem hiding this comment.
Hm, not totally sure - I read this particular comment https://stackoverflow.com/a/1436756/2213289 which seems to suggest you want the unique identifier in the repr.
python/ray/actor.py
Outdated
| @@ -841,7 +841,10 @@ def __getattribute__(self, attr): | |||
| return object.__getattribute__(self, attr) | |||
|
|
|||
| def __repr__(self): | |||
| return "Actor(" + self._ray_actor_id.hex() + ")" | |||
| return "Actor({})".format(self._ray_actor_id) | |||
There was a problem hiding this comment.
We probably want to keep the .hex(), right?
|
Ok, let's remove the |
|
Would a truncated ID be fine? Like first 6 of the hex? It helps
significantly with readability when debugging.
…On Sat, Jul 7, 2018 at 2:59 PM Robert Nishihara ***@***.***> wrote:
Ok, let's remove the __str__ implementation (it will just use __repr__)
and put both the class name and ID into the string. How does that sound?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2369 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEUc5Z1JGpmaH_ZTaUCCSJpU95_IZNolks5uES86gaJpZM4VGecU>
.
|
|
I'd prefer to keep the full ID so that in the future when we have a more fully-fledged UI, people can copy and paste it into the UI and get more information about the actor. The full ID should also help with searching through logs and the GCS and things like that. Also, we use full IDs for all of the other kinds of IDs, so it would be more consistent. |
|
Test FAILed. |
|
Test FAILed. |
|
this is ready for another pass @robertnishihara |
|
Test PASSed. |
This is a shot at a better string representation for actors.
Right now, the actor handle returned does not provide information about what class the actor handle is wrapping.
Suggestions and improvements welcome.
cc @stephanie-wang @robertnishihara @pcmoritz