Update Examples with the str.format() syntax. #870

SitiSchu · 2017-10-13T21:50:09Z

conversationbot.py:
Use str.format()

conversationbot2.py:
Use str.format()
make the states dict flake8 compatible.

echobot.py:
Change Docstring to a Docstring using triple quotes.
Add Docstrings to main() and echo()

echobot2.py:
Add File Description to Docstring
Add License notice to Docstring
Add Docstrings to start(), help(), echo() and main()

inlinebot.py:
Use str.format()
Add File Description to Docstring
Add License notice to Docstring
Add Docstring to start(), help(), inlinequery(), error()
Create the list "results" with the Inline Results instead of appending them.

inlinekeyboard.py:
Use str.format()
Put the Update in a main() function.

paymentbot.py:
Use str.format()
Shorten comment on how to get a provider token to avoid the comment being longer than 99 characters

timerbot.py:
Use str.format()
Add Docstrings to alarm(), set_timer(), unset()
Rename set() to set_timer() to avoud shadowing the built-in set()

conversationbot.py: Use `str.format()` conversationbot2.py: Use `str.format()` make the states dict flake8 compatible. echobot.py: Change Docstring to a Docstring using triple quotes. Add Docstrings to `main()` and `echo()` echobot2.py: Add File Description to Docstring Add License notice to Docstring Add Docstrings to `start()`, `help()`, `echo()` and `main()` inlinebot.py: Use `str.format()` Add File Description to Docstring Add License notice to Docstring Add Docstring to `start()`, `help()`, `inlinequery()`, `error()` Create the list "results" with the Inline Results instead of appending them. inlinekeyboard.py: Use `str.format()` Put the Update in a `main()` function. paymentbot.py: Use `str.format()` Shorten comment on how to get a provider token to avoid the comment being longer than 99 characters timerbot.py: Use `str.format()` Add Docstrings to `alarm()`, `set_timer()`, `unset()` Rename `set()` to `set_timer()` to avoud shadowing the built-in `set()`

codecov · 2017-10-14T01:14:08Z

Codecov Report

Merging #870 into master will increase coverage by 0.35%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #870      +/-   ##
==========================================
+ Coverage   91.44%   91.79%   +0.35%     
==========================================
  Files         101      101              
  Lines        3985     3985              
  Branches      603      603              
==========================================
+ Hits         3644     3658      +14     
+ Misses        195      189       -6     
+ Partials      146      138       -8

Flag	Coverage Δ
#Appveyor	`86.85% <ø> (+0.6%)`	⬆️
#Travis	`91.41% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
telegram/message.py	`96.01% <0%> (+4.78%)`	⬆️
telegram/games/game.py	`100% <0%> (+5.71%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a8b121...9e9b0bd. Read the comment docs.

Thanks to @KamranMackey for making me aware of the fact that Log messages shouldnt use str.format() according to this article: http://reinout.vanrees.org/weblog/2015/06/05/logging-formatting.html

tsnoam

Hi @SitiSchu
Thanks for your contribution. In general looks good (I like str.format()) but there are some changes which I must ask you to do. Please see CR comments on the code.

tsnoam · 2017-10-14T18:08:46Z

examples/conversationbot.py

@@ -46,7 +46,7 @@ def start(bot, update):

 def gender(bot, update):
    user = update.message.from_user
-    logger.info("Gender of %s: %s" % (user.first_name, update.message.text))
+    logger.info("Gender of {}: {}".format(user.first_name, update.message.text))


Good catch here, we shouldn't be formatting logs ourselves but rather let logging do it. Unfortunately logging only work with %s notation. So please change to:

logger.info("Gender of %s: %s", user.first_name, update.message.text)

edit: I see that part of the logging occurrences you've done that way.

tsnoam · 2017-10-14T18:09:04Z

examples/conversationbot.py

@@ -58,7 +58,7 @@ def photo(bot, update):
    user = update.message.from_user
    photo_file = bot.get_file(update.message.photo[-1].file_id)
    photo_file.download('user_photo.jpg')
-    logger.info("Photo of %s: %s" % (user.first_name, 'user_photo.jpg'))
+    logger.info("Photo of {}: {}".format(user.first_name, 'user_photo.jpg'))


same comment about logging

tsnoam · 2017-10-14T18:09:14Z

examples/conversationbot.py

@@ -67,7 +67,7 @@ def photo(bot, update):

 def skip_photo(bot, update):
    user = update.message.from_user
-    logger.info("User %s did not send a photo." % user.first_name)
+    logger.info("User {} did not send a photo.".format(user.first_name))


same comment about logging

tsnoam · 2017-10-14T18:09:23Z

examples/conversationbot.py

-    logger.info("Location of %s: %f / %f"
-                % (user.first_name, user_location.latitude, user_location.longitude))
+    logger.info("Location of {}: {} / {}".format(
+            user.first_name, user_location.latitude, user_location.longitude))


same comment about logging

tsnoam · 2017-10-14T18:10:31Z

examples/conversationbot2.py

-        reply_markup=markup)
+            "Hi! My name is Doctor Botter. I will hold a more complex conversation with you. "
+            "Why don't you tell me something about yourself?",
+            reply_markup=markup)


why did you change the indentation? please revert.

tsnoam · 2017-10-14T18:11:26Z

examples/conversationbot.py

    update.message.reply_text('Thank you! I hope we can talk again some day.')

    return ConversationHandler.END


 def cancel(bot, update):
    user = update.message.from_user
-    logger.info("User %s canceled the conversation." % user.first_name)
+    logger.info("User {} canceled the conversation.".format(user.first_name))


same comment about logging

tsnoam · 2017-10-14T18:19:52Z

examples/conversationbot2.py

+                },
+
+            fallbacks=[RegexHandler('^Done$', done, pass_user_data=True)]
+            )


why did you change the indentation here? please revert

tsnoam · 2017-10-14T18:21:49Z

examples/inlinebot.py

+                                        title="Italic",
+                                        input_message_content=InputTextMessageContent(
+                                                "_{}_".format(escape_markdown(query)),
+                                                parse_mode=ParseMode.MARKDOWN))]


why did you change the indentation here? please revert.

tsnoam · 2017-10-14T18:24:06Z

examples/paymentbot.py

@@ -31,7 +33,7 @@ def start_with_shipping_callback(bot, update):
    description = "Payment Example using python-telegram-bot"
    # select a payload just for you to recognize its the donation from your bot
    payload = "Custom-Payload"
-    # get your provider_token at @botfather, see https://core.telegram.org/bots/payments#getting-a-token
+    # to get your provider_token see https://core.telegram.org/bots/payments#getting-a-token


If we're into grammar, "In order to get a provider_token ..." would be more appropriate.

tsnoam · 2017-10-14T18:24:27Z

examples/paymentbot.py

@@ -55,7 +57,7 @@ def start_without_shipping_callback(bot, update):
    description = "Payment Example using python-telegram-bot"
    # select a payload just for you to recognize its the donation from your bot
    payload = "Custom-Payload"
-    # get your provider_token at @botfather, see https://core.telegram.org/bots/payments#getting-a-token
+    # check https://core.telegram.org/bots/payments#supported-currencies for more details


Same comment about grammar.

Removed the Extra Indent in the start() function.

codecov · 2017-10-14T23:08:17Z

Codecov Report

Merging #870 into master will increase coverage by 0.35%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #870      +/-   ##
==========================================
+ Coverage   91.44%   91.79%   +0.35%     
==========================================
  Files         101      101              
  Lines        3985     3985              
  Branches      603      603              
==========================================
+ Hits         3644     3658      +14     
+ Misses        195      189       -6     
+ Partials      146      138       -8

Flag	Coverage Δ
#Appveyor	`86.85% <ø> (+0.6%)`	⬆️
#Travis	`91.41% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
telegram/message.py	`96.01% <0%> (+4.78%)`	⬆️
telegram/games/game.py	`100% <0%> (+5.71%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a8b121...1d48a83. Read the comment docs.

codecov · 2017-10-14T23:08:18Z

Codecov Report

Merging #870 into master will increase coverage by 0.29%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #870      +/-   ##
==========================================
+ Coverage   91.44%   91.73%   +0.29%     
==========================================
  Files         101      101              
  Lines        3985     4055      +70     
  Branches      603      620      +17     
==========================================
+ Hits         3644     3720      +76     
- Misses        195      197       +2     
+ Partials      146      138       -8

Flag	Coverage Δ
#Appveyor	`86.88% <ø> (+0.63%)`	⬆️
#Travis	`91.31% <ø> (-0.1%)`	⬇️

Impacted Files	Coverage Δ
telegram/bot.py	`87.22% <0%> (-0.21%)`	⬇️
telegram/inline/inputlocationmessagecontent.py	`100% <0%> (ø)`	⬆️
telegram/inline/inlinequeryresultlocation.py	`100% <0%> (ø)`	⬆️
telegram/chat.py	`100% <0%> (ø)`	⬆️
telegram/message.py	`96.22% <0%> (+4.99%)`	⬆️
telegram/games/game.py	`100% <0%> (+5.71%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a8b121...7df6c08. Read the comment docs.

tsnoam

Thanks for all the changes. Unfortunately, I'm afraid I'll have to ask you a few more changes, mostly regarding indentation.

tsnoam · 2017-10-15T18:55:52Z

examples/conversationbot2.py

-                              reply_markup=markup)
+                              "{}"
+                              "You can tell me more, or change your opinion on something.".format(
+                               facts_to_str(user_data)), reply_markup=markup)


Indentation here still needs fixing.
Try using autopep8 on the file.

tsnoam · 2017-10-15T18:57:22Z

examples/conversationbot2.py

@@ -129,10 +130,10 @@ def main():
                                          received_information,
                                          pass_user_data=True),
                           ],
-        },
+            },


autopep8 will also fix indentation here.

tsnoam · 2017-10-15T18:57:29Z

examples/conversationbot2.py


        fallbacks=[RegexHandler('^Done$', done, pass_user_data=True)]
-    )
+        )


autopep8 will also fix indentation here.

tsnoam · 2017-10-15T19:00:58Z

examples/inlinebot.py

+                                 title="Italic",
+                                 input_message_content=InputTextMessageContent(
+                                     "_{}_".format(escape_markdown(query)),
+                                     parse_mode=ParseMode.MARKDOWN))]


autopep8 can be of assistant here as well.

autopep8 did this to that part:

This is how it looked before:

imho the way it was before is more readable

tsnoam · 2017-10-15T19:15:22Z

examples/paymentbot.py

@@ -55,7 +57,8 @@ def start_without_shipping_callback(bot, update):
    description = "Payment Example using python-telegram-bot"
    # select a payload just for you to recognize its the donation from your bot
    payload = "Custom-Payload"
-    # get your provider_token at @botfather, see https://core.telegram.org/bots/payments#getting-a-token
+    # In order to get a provider_token to get your provider_token see


In order to get a provider_token ~~to get your provider_token~~ see https://core.telegram.org/bots/payments#getting-a-token

Use autopep8 on the conversationbot2.py Fix the provide token comment (duplicated some words)

SitiSchu · 2017-10-15T20:59:08Z

Did all changes except the one to inlinebot.py:inlinequery() since I'm not sure if autopep8 makes it better.

SitiSchu · 2017-10-16T06:08:06Z

I formated the InlineQuery results in inlinebot.py like @jh0ker suggested with a linebreak before the id=uuid4()
This looks better and autopep8 didnt change the formatting to something weird.

jh0ker

replace logger.warn with logger.warning as per https://docs.python.org/3/library/logging.html#logging.Logger.warning

jh0ker · 2017-10-16T06:37:55Z

examples/conversationbot.py

    update.message.reply_text('Bye! I hope we can talk again some day.',
                              reply_markup=ReplyKeyboardRemove())

    return ConversationHandler.END


 def error(bot, update, error):
-    logger.warn('Update "%s" caused error "%s"' % (update, error))
+    logger.warn('Update "%s" caused error "%s"', update, error)


Can you change this to logger.warning? Logger.warn is deprecated.
https://docs.python.org/3/library/logging.html#logging.Logger.warning

jh0ker · 2017-10-16T06:38:09Z

examples/conversationbot2.py


    user_data.clear()
    return ConversationHandler.END


 def error(bot, update, error):
-    logger.warn('Update "%s" caused error "%s"' % (update, error))
+    logger.warn('Update "%s" caused error "%s"', update, error)


Can you change this to logger.warning? Logger.warn is deprecated.
https://docs.python.org/3/library/logging.html#logging.Logger.warning

jh0ker · 2017-10-16T06:38:17Z

examples/echobot2.py

    update.message.reply_text(update.message.text)


 def error(bot, update, error):
-    logger.warn('Update "%s" caused error "%s"' % (update, error))
+    logger.warn('Update "%s" caused error "%s"', update, error)


Can you change this to logger.warning? Logger.warn is deprecated.
https://docs.python.org/3/library/logging.html#logging.Logger.warning

jh0ker · 2017-10-16T06:38:30Z

examples/inlinebot.py


    update.inline_query.answer(results)


 def error(bot, update, error):
-    logger.warning('Update "%s" caused error "%s"' % (update, error))
+    logger.warn('Update "%s" caused error "%s"', update, error)


Can you change this back to logger.warning? Logger.warn is deprecated.
https://docs.python.org/3/library/logging.html#logging.Logger.warning

jh0ker · 2017-10-16T06:38:41Z

examples/inlinekeyboard.py

@@ -36,20 +36,26 @@ def help(bot, update):


 def error(bot, update, error):
-    logging.warning('Update "%s" caused error "%s"' % (update, error))
+    logger.warn('Update "%s" caused error "%s"', update, error)


Can you change this back to logger.warning? Logger.warn is deprecated.
https://docs.python.org/3/library/logging.html#logging.Logger.warning

jh0ker · 2017-10-16T06:38:51Z

examples/paymentbot.py


 logger = logging.getLogger(__name__)


 def error(bot, update, error):
-    logger.warn('Update "%s" caused error "%s"' % (update, error))
+    logger.warn('Update "%s" caused error "%s"', update, error)


Can you change this to logger.warning? Logger.warn is deprecated.
https://docs.python.org/3/library/logging.html#logging.Logger.warning

jh0ker · 2017-10-16T06:39:02Z

examples/timerbot.py

@@ -73,10 +73,11 @@ def unset(bot, update, chat_data):


 def error(bot, update, error):
-    logger.warning('Update "%s" caused error "%s"' % (update, error))
+    logger.warn('Update "%s" caused error "%s"', update, error)


Can you change this back to logger.warning? Logger.warn is deprecated.
https://docs.python.org/3/library/logging.html#logging.Logger.warning

jh0ker · 2017-10-16T06:40:21Z

@SitiSchu A bit more nitpicking :P

SitiSchu · 2017-10-16T08:38:41Z

@jh0ker PyCharm told me about this but I wasn't sure if Py2 supports it. I checked and it does support it so I'll change it today ^^.

jh0ker · 2017-10-16T08:38:59Z

@SitiSchu Awesome, thanks :)

SitiSchu · 2017-10-16T11:50:56Z

I updated all error functions to the new warning syntax. I also made flake8 pass(except D103 Missing docstring in public function)

jh0ker · 2017-10-16T13:38:52Z

examples/inlinebot.py

@@ -44,7 +45,7 @@ def help(bot, update):


 def escape_markdown(text):


Can we replace this as well? We have telegram.utils.helpers.escape_markdown now.

SitiSchu · 2017-10-16T14:34:00Z

@jh0ker Done.

SitiSchu · 2017-10-20T16:40:16Z

Any updates on this PR? :)

Eldinnie · 2017-10-20T16:45:53Z

LGTM @tsnoam any more changes requested?

tsnoam · 2017-10-20T18:24:11Z

@SitiSchu Thankyou for your contribution.

Use the "proper" way to format logging strings.

a261943

Thanks to @KamranMackey for making me aware of the fact that Log messages shouldnt use str.format() according to this article: http://reinout.vanrees.org/weblog/2015/06/05/logging-formatting.html

python-telegram-bot deleted a comment from codecov bot Oct 14, 2017

tsnoam requested changes Oct 14, 2017

View reviewed changes

Simon Schürrle added 3 commits October 15, 2017 01:00

Update paymentbot.py with requested changes.

25bf8e3

Revert Indentation in conversationbot2.py

5745997

Removed the Extra Indent in the start() function.

Use the proper way to format loggin strings in conversationbot.py

1d48a83

Simon Schürrle added 2 commits October 15, 2017 03:08

Revert Indentation in conversationbot2.py

a3c4059

Change Indentation in inlinebot.py

49096ed

tsnoam requested changes Oct 15, 2017

View reviewed changes

Simon Schürrle added 2 commits October 15, 2017 22:54

autopep8 convobot2, fix payment bot derp

adff025

Use autopep8 on the conversationbot2.py Fix the provide token comment (duplicated some words)

Fix both provider_token comments

3efc6b9

Used Jannes(jh0ker) suggestion for the inlinequery.py

9b6d857

jh0ker requested changes Oct 16, 2017

View reviewed changes

Change logger.warn to logger.warning, Fix flake8 errors(except D103)

c6d4133

jh0ker reviewed Oct 16, 2017

View reviewed changes

replace custom escape_markdown function in inlinebot.py with the builtin

7df6c08

jh0ker approved these changes Oct 16, 2017

View reviewed changes

tsnoam approved these changes Oct 20, 2017

View reviewed changes

tsnoam merged commit 38637ec into python-telegram-bot:master Oct 20, 2017

This was referenced Dec 8, 2017

Update python-telegram-bot to 9.0.0 freqtrade/freqtrade#168

Merged

Scheduled weekly dependency update for week 49 gpchelkin/scdlbot#107

Merged

pyup-bot mentioned this pull request Jan 22, 2018

Pin python-telegram-bot to latest version 9.0.0 skeledrew/ratot#2

Closed

This was referenced Mar 2, 2018

Pin python-telegram-bot to latest version 10.0.0 skeledrew/ratot#11

Closed

Pin python-telegram-bot to latest version 10.0.1 skeledrew/ratot#12

Merged

github-actions bot locked and limited conversation to collaborators Aug 22, 2020

		@@ -44,7 +45,7 @@ def help(bot, update):


		def escape_markdown(text):

Update Examples with the str.format() syntax. #870

Update Examples with the str.format() syntax. #870

Conversation

SitiSchu commented Oct 13, 2017

codecov bot commented Oct 14, 2017

Codecov Report

tsnoam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 14, 2017

Codecov Report

codecov bot commented Oct 14, 2017 • edited

Codecov Report

tsnoam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SitiSchu commented Oct 15, 2017

SitiSchu commented Oct 16, 2017 • edited

jh0ker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jh0ker commented Oct 16, 2017

SitiSchu commented Oct 16, 2017

jh0ker commented Oct 16, 2017

SitiSchu commented Oct 16, 2017

Choose a reason for hiding this comment

SitiSchu commented Oct 16, 2017

SitiSchu commented Oct 20, 2017

Eldinnie commented Oct 20, 2017

tsnoam commented Oct 20, 2017

codecov bot commented Oct 14, 2017 •

edited

SitiSchu commented Oct 16, 2017 •

edited