[FEATURE] Provide a "official" string representation for most classes

[harshil21]

What kind of feature are you missing? Where do you notice a shortcoming of PTB?

We should define a __repr__ for most telegram classes, since it is useful for debugging/inspection purposes.

Describe the solution you'd like

Define a __repr__ in all base classes. Typically, the representation should be of the form <...some useful description...>. We can just have it so that it returns the expression which can be used to recreate the object.

Describe alternatives you've considered

No response

Additional context

First suggested in #3759 (comment)

[CarlosZBent]

Hello @harshil21 , I'd like to take care of this.

[harshil21]

@CarlosZBent hi, at the moment the team would like to discuss this proposal. If we're going ahead with it, we'll notify you.

[CarlosZBent]

@CarlosZBent hi, at the moment the team would like to discuss this proposal. If we're going ahead with it, we'll notify you.

Sounds good, thanks for letting me know

[Bibo-Joshi]

A few initial toughts:

• Application: An association with the used bot would make sense. One can ofc think about adding all the other config values, but I don't see the additional value
• AppBuilder: Used exclusively for setup purposes. I don't see additional value in adding a representation
• CallbackContext: Associated update would be nice. For telegram.Update objects, the ID should suffice instead of the full representation (UPD it has been decided against implementing __repr__() for CCT for now)
• ContextTypes: Used exclusively for setup purposes. I don't see additional value in adding a representation
• Defaults: Used exclusively for setup purposes. I don't see additional value in adding a representation
• (Ext)Bot: class name and Token would make sense IMO
• Job: Job ID and callback name would make sense IMO. One can think about the schedule, too, but that may get long and I'm not so sure about the added value … edit: also the "name", if set. 2nd edit: After discussion below, schedule should be included
• JobQueue: Don't see much added value of having a repr here. If there is, it should probably contain the association to the application
• Updater: An association with the used bot would make sense
• Handlers:
  • In general Class name & callback name would make sense
  • For ConversationHandler, this is more difficult. If it has a name, it's easy, otherwise I don't think that printing the state handlers or entry points would make it better …
  • There are several handlers that have additional options (patterns, filters, …). However, the purpose of __repr__ is to be unambiguous and I do think that cases where you have instances of the same handler class with the same callback but different options are neglegible. Callback functions with the same name (module_1.start and module_2.start) may be more commen and we should probably try to print the fully qualified domain name …
• Persistence
  • {Base, Dict, Pickle}Persistence: I don't see added value of a nice repr. For PicklePersistence one could think about the file name, but one usually only has one persistence instance anyways …
  • PersistenceInput: Is i a namedtuple, should already have a nice repr
• Arb. CallbackData
  • CallbackDataCache: Don't see a need for a repr
• Rate Limiting
  • {Base, AIORateLimiter}: I don't see added value of a nice repr, as one usually only has one instance anyways.

As a general thought: __repr__ should work independently of whether an object was already initalize() -ed or not. So it must not depend on info that's only gathered in initialize or at very least provide an alternative representation. I strongly prefer the "independent" option though.

[lemontree210]

My impression is that in terms of the common understanding of the __repr__ vs. __str__ dichotomy, in most cases you're talking about __str__ rather than __repr__ here.

[harshil21]

I also feel the same as @lemontree210. __repr__ should output a representation which can be used to recreate the object again. While this may not be directly possible for some cases, we should try to support it.

Also just remembered that adding a __repr__ is also useful when using PTB in a Juptyer/IPython environment.

AppBuilder: Used exclusively for setup purposes. I don't see additional value in adding a representation

For complex setups, it can be useful to print the builder and see what has been already added.

[Bibo-Joshi]

repr should output a representation which can be used to recreate the object again.

I disagree with that. Providing an (almost) unambiguous representation doesn't necessarily coincide with being able to recreate the object. E.g. if an object has a unique ID, printing that ID suffices for unambiguous representation (end hence pythons default implementation uses id(…)).

Also just remembered that adding a repr is also useful when using PTB in a Juptyer/IPython environment.

How so?

For complex setups, it can be useful to print the builder and see what has been already added.

Would you mind elaborating this use case? Bots are rarely run in an interactive shell, so I would expect that if you wanted to inspect the builder, you'd rather have to do that automatically at runtime and for that parsing string representations seems like an insane approach …

[harshil21]

How so?
Would you mind elaborating

For users new to a particular function / the library itself, it can be useful to see what the object is, like what attributes it has.
For the builder case, consider a script which has to determine a) where it is being run and/or b) any command line args passed so it can build the application, with local_mode, maybe a proxy, timeouts, etc. Ofc this is mostly only useful at coding time / logging purposes.

E.g. if an object has a unique ID

Ok, I don't mind showing only certain attributes in the representation, anything is better than the default, which is nothing useful imo.

parsing string representations seems like an insane

you mean implementing __repr__ for AppBuilder is insane? I don't expect the users to parse the returned string, its just for viewing purposes.

I agree with your list of which classes should get a repr btw (except appbuilder ^)

strongly prefer the "independent" option though

✅

[Bibo-Joshi]

The "How so?" was about why string representations are especially useful for jupyter/Ipython :)

you mean implementing repr for AppBuilder is insane?

I mean that parsing a string representation of AppBuilder to find out the settings would be insane. But yes, I also think that implementing a meaningful representation could be harder than for other classes

For the builder case, consider a script which has to determine a) where it is being run and/or b) any command line args passed so it can build the application, with local_mode, maybe a proxy, timeouts, etc. Ofc this is mostly only useful at coding time / logging purposes.

Sorry, I still don't see how a custom string representation of ApplictanBuilder would help with these use cases.

agree with your list of which classes should get a repr btw (except appbuilder ^)

❤️

[lemontree210]

I like the following quote from an SO discussion:

If you have enough information so eval(repr(c))==c, that means you know everything there is to know about c. If that’s easy enough, at least in a fuzzy way, do it. If not, make sure you have enough information about c anyway. I usually use an eval-like format: "MyClass(this=%r,that=%r)" % (self.this,self.that). It does not mean that you can actually construct MyClass, or that those are the right constructor arguments — but it is a useful form to express “this is everything you need to know about this instance”.

Take e.g. Updater. Is the associated bot really everything a developer needs to know about the Updater when logging/debugging? Or take Job: if we don't include the schedule, that's probably not everything a developer needs to know about it.

I would also rather agree with the same SO reply saying that virtually any class has to have a __repr__, but this is probably not realistic, at least not now.

I would still argue that in some cases you're rather talking about __str__ and not __repr__. By the way, even in the #3759 comment, the importance of readability is mentioned, which actually leads to creating __str__ methods for selected classes.

[Bibo-Joshi]

I would still argue that in some cases you're rather talking about str and not repr. By the way, even in the #3759 (comment), the importance of readability is mentioned, which actually leads to creating str methods for selected classes.

IMO the difference between __str__ and __repr__ mostly shows for things that you want a consumer of software to see. E.g. if you have a class DinnerMenu, then DinnerMenu.__str__ (to be used in user-output) should probably be very different from DinnerMenu.__repr__ (to be used in logs etc). For the things we're discussion here, the reader of both __str__ and __repr__ should usually be the developer of the bot or at the most other developers. I would hence argue that it's okay for __str__ to just give the same output as __repr__ and to think about what we expect __repr__ to do.
This basically coincides with the summary of the cited SO discussion:

Implement repr for any class you implement. This should be second nature. Implement str if you think it would be useful to have a string version which errs on the side of readability.

---

Take e.g. Updater. Is the associated bot really everything a developer needs to know about the Updater when logging/debugging? Or take Job: if we don't include the schedule, that's probably not everything a developer needs to know about it.

Let's maybe clarify what our views on "needs to know" are :) From my above argumentation, what a developer needs to know about an object (in the logs/debug output) is the info that they need to identify the object (uniquely, at best) and to know which places in the code to look for when trying to fix exceptions.

For the Updater, you usually only have one object anyways and its configuration is not changed during runtime. So even if you have several Updater instances, knowing which which bot they belong to should suffice to look it up in your code for most every case.
Now, jobs are usually created dynamically during runtime and I agree that for such cases more information is useful. I'm okay with adding the schedule. I alsod edited my above list to also include job.name, if set.

[lemontree210]

I know what you mean, it's somewhat strange that in our case the line between "users" and "developers" is sort of blurred :)

Implement repr for any class you implement. This should be second nature.

This one really got my attention because we're thinking of not adding __repr__ at all for some classes whereas the author thinks we should add __repr__ for all of them.

Maybe the intermediate approach would be to find classes that we want to show different info about for PTB developers ("developers") as opposed to bot developers that use PTB ("users"). These classes might show a lot of debug information in __repr__ and only something informative for the bot developer in __str__. In other cases, just implement __repr__. Maybe, of course, even this intermediate approach is not needed since our tests seem to be exhaustive and they eliminate the need for very elaborate __repr__s for "developers".

[Bibo-Joshi]

__repr__ and __str__ are buth well-known methods of python objects. Overriding one fo them to be tailored only for the PTB team sounds like an unwise move to me, tbh, and I would expect that to cause a lot of confusion

[harshil21]

I agree with Hinrich on only overriding __repr__ and not __str__ since the latter's use case is limited and not much different from the former in our case imo.

[Bibo-Joshi]

@CarlosZBent I guess the discussions have been resolved now. Are you sitll interested in PRing for this?

[CarlosZBent]

@Bibo-Joshi I'm sorry but no, I'm unable at this moment.

[CarlosZBent]

@Bibo-Joshi I'm sorry but no, I'm unable at this moment.