From e2c6d60721f2fb747c4efce5bf5b468043d6bbdb Mon Sep 17 00:00:00 2001 From: Bibo-Joshi Date: Thu, 27 May 2021 20:34:58 +0200 Subject: [PATCH] Improve Code Quality (#2536) * Start fixing stuff * More docstrings * stabilize test_idle Co-authored-by: Harshil --- examples/chatmemberbot.py | 3 ++- examples/conversationbot.py | 9 +++++++++ examples/conversationbot2.py | 13 ++++++++----- examples/errorhandlerbot.py | 6 +++--- examples/inlinebot.py | 1 + examples/inlinekeyboard.py | 4 ++++ examples/inlinekeyboard2.py | 4 +++- examples/nestedconversationbot.py | 1 + examples/paymentbot.py | 18 +++++++++++------- examples/persistentconversationbot.py | 20 +++++++++++--------- examples/pollbot.py | 1 + examples/rawapibot.py | 9 +++++---- examples/timerbot.py | 1 + tests/test_updater.py | 2 +- 14 files changed, 61 insertions(+), 31 deletions(-) diff --git a/examples/chatmemberbot.py b/examples/chatmemberbot.py index dd54c06de..5ed86815d 100644 --- a/examples/chatmemberbot.py +++ b/examples/chatmemberbot.py @@ -35,7 +35,8 @@ def extract_status_change( ) -> Optional[Tuple[bool, bool]]: """Takes a ChatMemberUpdated instance and extracts whether the 'old_chat_member' was a member of the chat and whether the 'new_chat_member' is a member of the chat. Returns None, if - the status didn't change.""" + the status didn't change. + """ status_change = chat_member_update.difference().get("status") old_is_member, new_is_member = chat_member_update.difference().get("is_member", (None, None)) diff --git a/examples/conversationbot.py b/examples/conversationbot.py index 9d6e1acf6..34233f324 100644 --- a/examples/conversationbot.py +++ b/examples/conversationbot.py @@ -37,6 +37,7 @@ GENDER, PHOTO, LOCATION, BIO = range(4) def start(update: Update, _: CallbackContext) -> int: + """Starts the conversation and asks the user about their gender.""" reply_keyboard = [['Boy', 'Girl', 'Other']] update.message.reply_text( @@ -50,6 +51,7 @@ def start(update: Update, _: CallbackContext) -> int: def gender(update: Update, _: CallbackContext) -> int: + """Stores the selected gender and asks for a photo.""" user = update.message.from_user logger.info("Gender of %s: %s", user.first_name, update.message.text) update.message.reply_text( @@ -62,6 +64,7 @@ def gender(update: Update, _: CallbackContext) -> int: def photo(update: Update, _: CallbackContext) -> int: + """Stores the photo and asks for a location.""" user = update.message.from_user photo_file = update.message.photo[-1].get_file() photo_file.download('user_photo.jpg') @@ -74,6 +77,7 @@ def photo(update: Update, _: CallbackContext) -> int: def skip_photo(update: Update, _: CallbackContext) -> int: + """Skips the photo and asks for a location.""" user = update.message.from_user logger.info("User %s did not send a photo.", user.first_name) update.message.reply_text( @@ -84,6 +88,7 @@ def skip_photo(update: Update, _: CallbackContext) -> int: def location(update: Update, _: CallbackContext) -> int: + """Stores the location and asks for some info about the user.""" user = update.message.from_user user_location = update.message.location logger.info( @@ -97,6 +102,7 @@ def location(update: Update, _: CallbackContext) -> int: def skip_location(update: Update, _: CallbackContext) -> int: + """Skips the location and asks for info about the user.""" user = update.message.from_user logger.info("User %s did not send a location.", user.first_name) update.message.reply_text( @@ -107,6 +113,7 @@ def skip_location(update: Update, _: CallbackContext) -> int: def bio(update: Update, _: CallbackContext) -> int: + """Stores the info about the user and ends the conversation.""" user = update.message.from_user logger.info("Bio of %s: %s", user.first_name, update.message.text) update.message.reply_text('Thank you! I hope we can talk again some day.') @@ -115,6 +122,7 @@ def bio(update: Update, _: CallbackContext) -> int: def cancel(update: Update, _: CallbackContext) -> int: + """Cancels and ends the conversation.""" user = update.message.from_user logger.info("User %s canceled the conversation.", user.first_name) update.message.reply_text( @@ -125,6 +133,7 @@ def cancel(update: Update, _: CallbackContext) -> int: def main() -> None: + """Run the bot.""" # Create the Updater and pass it your bot's token. updater = Updater("TOKEN") diff --git a/examples/conversationbot2.py b/examples/conversationbot2.py index 98d45aef8..cab9d6a0c 100644 --- a/examples/conversationbot2.py +++ b/examples/conversationbot2.py @@ -45,15 +45,13 @@ markup = ReplyKeyboardMarkup(reply_keyboard, one_time_keyboard=True) def facts_to_str(user_data: Dict[str, str]) -> str: - facts = list() - - for key, value in user_data.items(): - facts.append(f'{key} - {value}') - + """Helper function for formatting the gathered user info.""" + facts = [f'{key} - {value}' for key, value in user_data.items()] return "\n".join(facts).join(['\n', '\n']) def start(update: Update, _: CallbackContext) -> int: + """Start the conversation and ask user for input.""" update.message.reply_text( "Hi! My name is Doctor Botter. I will hold a more complex conversation with you. " "Why don't you tell me something about yourself?", @@ -64,6 +62,7 @@ def start(update: Update, _: CallbackContext) -> int: def regular_choice(update: Update, context: CallbackContext) -> int: + """Ask the user for info about the selected predefined choice.""" text = update.message.text context.user_data['choice'] = text update.message.reply_text(f'Your {text.lower()}? Yes, I would love to hear about that!') @@ -72,6 +71,7 @@ def regular_choice(update: Update, context: CallbackContext) -> int: def custom_choice(update: Update, _: CallbackContext) -> int: + """Ask the user for a description of a custom category.""" update.message.reply_text( 'Alright, please send me the category first, for example "Most impressive skill"' ) @@ -80,6 +80,7 @@ def custom_choice(update: Update, _: CallbackContext) -> int: def received_information(update: Update, context: CallbackContext) -> int: + """Store info provided by user and ask for the next category.""" user_data = context.user_data text = update.message.text category = user_data['choice'] @@ -97,6 +98,7 @@ def received_information(update: Update, context: CallbackContext) -> int: def done(update: Update, context: CallbackContext) -> int: + """Display the gathered info and end the conversation.""" user_data = context.user_data if 'choice' in user_data: del user_data['choice'] @@ -111,6 +113,7 @@ def done(update: Update, context: CallbackContext) -> int: def main() -> None: + """Run the bot.""" # Create the Updater and pass it your bot's token. updater = Updater("TOKEN") diff --git a/examples/errorhandlerbot.py b/examples/errorhandlerbot.py index f11cee118..1ca3ff018 100644 --- a/examples/errorhandlerbot.py +++ b/examples/errorhandlerbot.py @@ -2,9 +2,7 @@ # pylint: disable=C0116 # This program is dedicated to the public domain under the CC0 license. -""" -This is a very simple example on how one could implement a custom error handler -""" +"""This is a very simple example on how one could implement a custom error handler.""" import html import json import logging @@ -59,6 +57,7 @@ def bad_command(_: Update, context: CallbackContext) -> None: def start(update: Update, _: CallbackContext) -> None: + """Displays info on how to trigger an error.""" update.effective_message.reply_html( 'Use /bad_command to cause an error.\n' f'Your chat id is {update.effective_chat.id}.' @@ -66,6 +65,7 @@ def start(update: Update, _: CallbackContext) -> None: def main() -> None: + """Run the bot.""" # Create the Updater and pass it your bot's token. updater = Updater(BOT_TOKEN) diff --git a/examples/inlinebot.py b/examples/inlinebot.py index 62e04f12e..00a11e25b 100644 --- a/examples/inlinebot.py +++ b/examples/inlinebot.py @@ -72,6 +72,7 @@ def inlinequery(update: Update, _: CallbackContext) -> None: def main() -> None: + """Run the bot.""" # Create the Updater and pass it your bot's token. updater = Updater("TOKEN") diff --git a/examples/inlinekeyboard.py b/examples/inlinekeyboard.py index 1305dc106..866da37f3 100644 --- a/examples/inlinekeyboard.py +++ b/examples/inlinekeyboard.py @@ -18,6 +18,7 @@ logger = logging.getLogger(__name__) def start(update: Update, _: CallbackContext) -> None: + """Sends a message with three inline buttons attached.""" keyboard = [ [ InlineKeyboardButton("Option 1", callback_data='1'), @@ -32,6 +33,7 @@ def start(update: Update, _: CallbackContext) -> None: def button(update: Update, _: CallbackContext) -> None: + """Parses the CallbackQuery and updates the message text.""" query = update.callback_query # CallbackQueries need to be answered, even if no notification to the user is needed @@ -42,10 +44,12 @@ def button(update: Update, _: CallbackContext) -> None: def help_command(update: Update, _: CallbackContext) -> None: + """Displays info on how to use the bot.""" update.message.reply_text("Use /start to test this bot.") def main() -> None: + """Run the bot.""" # Create the Updater and pass it your bot's token. updater = Updater("TOKEN") diff --git a/examples/inlinekeyboard2.py b/examples/inlinekeyboard2.py index fa16c4073..0ce5bb73d 100644 --- a/examples/inlinekeyboard2.py +++ b/examples/inlinekeyboard2.py @@ -151,7 +151,8 @@ def four(update: Update, _: CallbackContext) -> int: def end(update: Update, _: CallbackContext) -> int: """Returns `ConversationHandler.END`, which tells the - ConversationHandler that the conversation is over""" + ConversationHandler that the conversation is over. + """ query = update.callback_query query.answer() query.edit_message_text(text="See you next time!") @@ -159,6 +160,7 @@ def end(update: Update, _: CallbackContext) -> int: def main() -> None: + """Run the bot.""" # Create the Updater and pass it your bot's token. updater = Updater("TOKEN") diff --git a/examples/nestedconversationbot.py b/examples/nestedconversationbot.py index 0ce9eee01..e24532bb1 100644 --- a/examples/nestedconversationbot.py +++ b/examples/nestedconversationbot.py @@ -301,6 +301,7 @@ def stop_nested(update: Update, _: CallbackContext) -> str: def main() -> None: + """Run the bot.""" # Create the Updater and pass it your bot's token. updater = Updater("TOKEN") diff --git a/examples/paymentbot.py b/examples/paymentbot.py index 0ba8a91c3..a1af9c07f 100644 --- a/examples/paymentbot.py +++ b/examples/paymentbot.py @@ -2,9 +2,7 @@ # pylint: disable=C0116 # This program is dedicated to the public domain under the CC0 license. -""" -Basic example for a bot that can receive payment from user. -""" +"""Basic example for a bot that can receive payment from user.""" import logging @@ -28,6 +26,7 @@ logger = logging.getLogger(__name__) def start_callback(update: Update, _: CallbackContext) -> None: + """Displays info on how to use the bot.""" msg = ( "Use /shipping to get an invoice for shipping-payment, or /noshipping for an " "invoice without shipping." @@ -37,6 +36,7 @@ def start_callback(update: Update, _: CallbackContext) -> None: def start_with_shipping_callback(update: Update, context: CallbackContext) -> None: + """Sends an invoice with shipping-payment.""" chat_id = update.message.chat_id title = "Payment Example" description = "Payment Example using python-telegram-bot" @@ -70,6 +70,7 @@ def start_with_shipping_callback(update: Update, context: CallbackContext) -> No def start_without_shipping_callback(update: Update, context: CallbackContext) -> None: + """Sends an invoice without shipping-payment.""" chat_id = update.message.chat_id title = "Payment Example" description = "Payment Example using python-telegram-bot" @@ -91,6 +92,7 @@ def start_without_shipping_callback(update: Update, context: CallbackContext) -> def shipping_callback(update: Update, _: CallbackContext) -> None: + """Answers the ShippingQuery with ShippingOptions""" query = update.shipping_query # check the payload, is this from your bot? if query.invoice_payload != 'Custom-Payload': @@ -98,10 +100,9 @@ def shipping_callback(update: Update, _: CallbackContext) -> None: query.answer(ok=False, error_message="Something went wrong...") return - options = list() - # a single LabeledPrice - options.append(ShippingOption('1', 'Shipping Option A', [LabeledPrice('A', 100)])) - # an array of LabeledPrice objects + # First option has a single LabeledPrice + options = [ShippingOption('1', 'Shipping Option A', [LabeledPrice('A', 100)])] + # second option has an array of LabeledPrice objects price_list = [LabeledPrice('B1', 150), LabeledPrice('B2', 200)] options.append(ShippingOption('2', 'Shipping Option B', price_list)) query.answer(ok=True, shipping_options=options) @@ -109,6 +110,7 @@ def shipping_callback(update: Update, _: CallbackContext) -> None: # after (optional) shipping, it's the pre-checkout def precheckout_callback(update: Update, _: CallbackContext) -> None: + """Answers the PreQecheckoutQuery""" query = update.pre_checkout_query # check the payload, is this from your bot? if query.invoice_payload != 'Custom-Payload': @@ -120,11 +122,13 @@ def precheckout_callback(update: Update, _: CallbackContext) -> None: # finally, after contacting the payment provider... def successful_payment_callback(update: Update, _: CallbackContext) -> None: + """Confirms the successful payment.""" # do something after successfully receiving payment? update.message.reply_text("Thank you for your payment!") def main() -> None: + """Run the bot.""" # Create the Updater and pass it your bot's token. updater = Updater("TOKEN") diff --git a/examples/persistentconversationbot.py b/examples/persistentconversationbot.py index 3022a449f..30a6df8ef 100644 --- a/examples/persistentconversationbot.py +++ b/examples/persistentconversationbot.py @@ -28,7 +28,6 @@ from telegram.ext import ( CallbackContext, ) - # Enable logging logging.basicConfig( format='%(asctime)s - %(name)s - %(levelname)s - %(message)s', level=logging.INFO @@ -47,15 +46,13 @@ markup = ReplyKeyboardMarkup(reply_keyboard, one_time_keyboard=True) def facts_to_str(user_data: Dict[str, str]) -> str: - facts = [] - - for key, value in user_data.items(): - facts.append(f'{key} - {value}') - + """Helper function for formatting the gathered user info.""" + facts = [f'{key} - {value}' for key, value in user_data.items()] return "\n".join(facts).join(['\n', '\n']) def start(update: Update, context: CallbackContext) -> int: + """Start the conversation, display any stored data and ask user for input.""" reply_text = "Hi! My name is Doctor Botter." if context.user_data: reply_text += ( @@ -73,6 +70,7 @@ def start(update: Update, context: CallbackContext) -> int: def regular_choice(update: Update, context: CallbackContext) -> int: + """Ask the user for info about the selected predefined choice.""" text = update.message.text.lower() context.user_data['choice'] = text if context.user_data.get(text): @@ -87,6 +85,7 @@ def regular_choice(update: Update, context: CallbackContext) -> int: def custom_choice(update: Update, _: CallbackContext) -> int: + """Ask the user for a description of a custom category.""" update.message.reply_text( 'Alright, please send me the category first, for example "Most impressive skill"' ) @@ -95,6 +94,7 @@ def custom_choice(update: Update, _: CallbackContext) -> int: def received_information(update: Update, context: CallbackContext) -> int: + """Store info provided by user and ask for the next category.""" text = update.message.text category = context.user_data['choice'] context.user_data[category] = text.lower() @@ -103,8 +103,7 @@ def received_information(update: Update, context: CallbackContext) -> int: update.message.reply_text( "Neat! Just so you know, this is what you already told me:" f"{facts_to_str(context.user_data)}" - "You can tell me more, or change your opinion on " - "something.", + "You can tell me more, or change your opinion on something.", reply_markup=markup, ) @@ -112,23 +111,26 @@ def received_information(update: Update, context: CallbackContext) -> int: def show_data(update: Update, context: CallbackContext) -> None: + """Display the gathered info.""" update.message.reply_text( f"This is what you already told me: {facts_to_str(context.user_data)}" ) def done(update: Update, context: CallbackContext) -> int: + """Display the gathered info and end the conversation.""" if 'choice' in context.user_data: del context.user_data['choice'] update.message.reply_text( - "I learned these facts about you:" f"{facts_to_str(context.user_data)}Until next time!", + f"I learned these facts about you: {facts_to_str(context.user_data)}Until next time!", reply_markup=ReplyKeyboardRemove(), ) return ConversationHandler.END def main() -> None: + """Run the bot.""" # Create the Updater and pass it your bot's token. persistence = PicklePersistence(filename='conversationbot') updater = Updater("TOKEN", persistence=persistence) diff --git a/examples/pollbot.py b/examples/pollbot.py index e2ea0f112..58c9a649e 100644 --- a/examples/pollbot.py +++ b/examples/pollbot.py @@ -151,6 +151,7 @@ def help_handler(update: Update, _: CallbackContext) -> None: def main() -> None: + """Run bot.""" # Create the Updater and pass it your bot's token. updater = Updater("TOKEN") dispatcher = updater.dispatcher diff --git a/examples/rawapibot.py b/examples/rawapibot.py index 5aac795ba..fed61b3d6 100644 --- a/examples/rawapibot.py +++ b/examples/rawapibot.py @@ -49,10 +49,11 @@ def echo(bot: telegram.Bot) -> None: for update in bot.get_updates(offset=UPDATE_ID, timeout=10): UPDATE_ID = update.update_id + 1 - if update.message: # your bot can receive updates without messages - if update.message.text: # not all messages contain text - # Reply to the message - update.message.reply_text(update.message.text) + # your bot can receive updates without messages + # and not all messages contain text + if update.message and update.message.text: + # Reply to the message + update.message.reply_text(update.message.text) if __name__ == '__main__': diff --git a/examples/timerbot.py b/examples/timerbot.py index 045e5ff45..12c41471b 100644 --- a/examples/timerbot.py +++ b/examples/timerbot.py @@ -34,6 +34,7 @@ logger = logging.getLogger(__name__) # Define a few command handlers. These usually take the two arguments update and # context. Error handlers also receive the raised TelegramError object in error. def start(update: Update, _: CallbackContext) -> None: + """Sends explanation on how to use the bot.""" update.message.reply_text('Hi! Use /set to set a timer') diff --git a/tests/test_updater.py b/tests/test_updater.py index 388b7f803..f9dc0ceac 100644 --- a/tests/test_updater.py +++ b/tests/test_updater.py @@ -484,7 +484,7 @@ class TestUpdater: # There is a chance of a conflict when getting updates since there can be many tests # (bots) running simultaneously while testing in github actions. - if caplog.records[-1].getMessage().startswith('Error while getting Updates: Conflict'): + if caplog.records[0].getMessage().startswith('Error while getting Updates: Conflict'): caplog.records.pop() # For stability rec = caplog.records[-2]