Initial commit #119
Initial commit #119
Conversation
| ### This module contains extended keyboard classes with extra functionality for PTB keyboards. | ||
|
|
||
| ### Methods is self-descriptive. |
There was a problem hiding this comment.
| ### This module contains extended keyboard classes with extra functionality for PTB keyboards. | |
| ### Methods is self-descriptive. | |
| This module contains extended keyboard classes with extra functionality for PTB keyboards. | |
| Methods are self-descriptive. |
These are no headlines :)
| if TYPE_CHECKING: | ||
| pass |
There was a problem hiding this comment.
| if TYPE_CHECKING: | |
| pass |
| class IExtendedInlineKeyboardMarkup( | ||
| ABC, | ||
| ): |
There was a problem hiding this comment.
| class IExtendedInlineKeyboardMarkup( | |
| ABC, | |
| ): | |
| class IExtendedInlineKeyboardMarkup(ABC): |
what does the leading "I" stand for? Can't be "inline" b/c that's already included later on :D
Also what's the purpose of providing this abstract base class? You provide exactly one implementation so you could just drop the base class. Note that you don't even make this class public as it's not declared in __init__
There was a problem hiding this comment.
"I" - Interface.
The interface is here just in case, some maintainers sometimes require them, depend on the ptbcontrlib policy :)
There was a problem hiding this comment.
Ah okay. This reminds me of C++ style. I personally don't see much benefit of it...
| ): | ||
| """Popular keyboard actions""" | ||
|
|
||
| class Exceptions: |
There was a problem hiding this comment.
what's the benefit of grouping the exceptions NotEnoughButtons and EmptyRowsDisallowed in the class Exceptions? and why are they defined only within the namespace of ExtendedInlineKeyboardMarkup? This makes it hard for a user to write somthing like
try:
# foo
except NotEnoughButtons:
# handle the exceptionThere was a problem hiding this comment.
Good point, I will put it outside.
| if strict: | ||
| condition = cbk == self.inline_keyboard[row_index][column_index].callback_data | ||
| else: # "in" to keep ability to match by prefix and some cbk key if need it | ||
| condition = cbk in self.inline_keyboard[row_index][column_index].callback_data |
There was a problem hiding this comment.
this will throw an exception in case callback_data is not a string, which can be the case if arbitrary callback_data is used. Please consider handling that case or at least documenting this limitation.
| "Result num of rows less that original num of rows in the keyboard." | ||
| ) | ||
|
|
||
| def to_list( |
There was a problem hiding this comment.
| def to_list( | |
| def to_nested_list( |
?
| keep_empty_rows: bool - keep empty rows in final keyboard if not enough buttons. | ||
| # Please create feature issue if you need it. |
There was a problem hiding this comment.
I'd be surprised if TG would accept this as input
|
|
||
| ### Methods is self-descriptive. | ||
|
|
||
| ```python |
There was a problem hiding this comment.
Here you copy-pasted the abstract base class from keyboards.py. However this doesn't tell a user how to use your contribution. Please remove this snippet and instead provide examples of how you envision this contribution to be applied in an PTB-based bot. Have a look at the other contributions for inspiration.
|
@david-shiko are you still interested in working on this? |
This module contains extended keyboard classes with extra functionality for PTB keyboards.
This is the first basic implementation, next releases will contain more advantage features.