Skip to content
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

[FEATURE] class-based nested conversations #2827

Closed
evakdev opened this issue Dec 24, 2021 · 4 comments
Closed

[FEATURE] class-based nested conversations #2827

evakdev opened this issue Dec 24, 2021 · 4 comments

Comments

@evakdev
Copy link

evakdev commented Dec 24, 2021

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

I opened a similar issue some time ago, but it got closed because of my inactivity. I'm gonna try to open it up for discussion again, this time hoping to provide more details :)

In the current function-based way of making bots (which is the default way in the examples provided), it can get very complex and messy to make nested conversations.
Imagine a menu with a nested conversation for each button, and sometimes another nested conversation inside one of those. It gets quite confusing to find where each function is, and its hard to organize things in different files without running to circular import issues.

Describe the solution you'd like

I believe having a class called Conversation could help. It would basically be some kind of wrapper for all the functions of the conversation, plus its ConversationHandler.

Possible features and benefits:

  • It could just get the arguments to ConversationHandler, and create the handler itself. So more concise and readable code.
  • It could include a place to keep all the state keys for that specific conversation, so you dont have to have a million keys for different convos jumbled up. they would be both organized and easy to find.
  • It could have a name or id attribute, and could add that to the beginning of every state key, making all the keys unique and less prone to mixups.
  • it could have predefined state keys, which would remove the hassle of thinking about state key names.(e.g key1,key2, etc) Because its in a class, they would be organized. and if you use the name/id idea above, they would also be unique.
  • it would provide a structure for nested conversations.
  • With some more work, it could be a blackbox for creating nested conversations with ease, just by defining the functions, the handlers and being done.

I think this could potentially be time-consuming (depending on how big one would go for it) and might need some things to change in the library, because it's kinda different from how ptb is currently set up. But if implemented, it could help a lot for making nested conversations.

Describe alternatives you've considered

No response

Additional context

Maybe people are already using ptb conversations within classes, I dont know. but if that's the way everyone does it, then the examples should reflect that. In any case, I think ptb could benefit from being more class-friendly in nested conversations :)

@Bibo-Joshi
Copy link
Member

Hi. Thanks for elaborating. Since improvements ideas for CH keep coming in and we're currently concentrating on the asyncio switch, I'll add this to #2770 for now so that we can come back to it later :)

That said, could you try and explain more detaild what you mean by predefined states and how class-based CHs could/would improve handling of nested conversations? I didin't really get those points.

@evakdev
Copy link
Author

evakdev commented Jan 2, 2022

Oops, seems like I missed this message @Bibo-Joshi. Sure, let me elaborate:

how class-based CHs could/would improve handling of nested conversations?
First of all, it's just easier to see what's going on when everything is inside classes. Conversation handling is a bit confusing but nested conversations are even more confusing.
Nested conversations also have the issue of returning to the parent correctly, which can be a bit hard to debug. For example, what do you do if you have a third level nested conversation and you want to return to first level from that? How do you handle having a main menu that user can return to from inside any of its nested conversations? You really have to think what to map, it gets tricky.
If there is a class, one could have some extra functions that handle this for you automatically. However, I'm aware that this part can take much more tweaking so maybe it's just something to think about for later :)

Predefined states:
let's say we're building the conversation handler example in examples, where we have:
GENDER, PHOTO, LOCATION, BIO = range(4)
There are two problems with this method:

  1. if you have several conversations, you have to worry about them being unique and not getting mixed up.
  2. Sometimes its not clear what they should be named. like maybe I'm asking for location in one state, and returning another location in another, and validating it in the third. it can be confusing to find the best names.
    If we had a class for the conversation handler which has something like state1, state2, state3,... then both of these are solved. they don't have to be unique because they are in a class, and you don't need to be concerned with naming because you could just go to state1 in step1 of convo, 2 in step 2 of the convo and so on.
    This could also be generated automatically as some kind of more complex key, so that the user doesn't have to worry about uniqueness even inside the handler.

@Bibo-Joshi
Copy link
Member

Thanks for getting elaborating. However, I'm still having troubles to understand how classes would help with the things that you mention.


You really have to think what to map, it gets tricky.

how would wrapping nested conversations into class-based conversationhandlers change that?

If there is a class, one could have some extra functions that handle this for you automatically.

Can you explain what kind of extra functions you are thinking of and what you would expect them to do?


if you have several conversations, you have to worry about them being unique and not getting mixed up.

Not necessarily. States just need to be unique per conversation (because that's just how dicts work …), not globally across conversations.

Sometimes its not clear what they should be named.

IMO that's more a question of design/taste, i.e. how you personally would like your states to be named

something like state1, state2, state3,...

you can already name your states state1, state2, state3,... though …

@Bibo-Joshi
Copy link
Member

closing due to inactivity

@github-actions github-actions bot locked and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants