Update _message.py - issue #2606 #2741

nivramam · 2021-10-21T02:26:57Z

Checklist for PR

modified message.photo = None instead of an empty list []
Added .. versionadded:: version, .. versionchanged:: version or .. deprecated:: version to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)
Created new or adapted existing unit tests
Added myself alphabetically to AUTHORS.rst (optional)

…ryptionError (python-telegram-bot#2621)

…t#2612) Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>

Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>

Signed-off-by: starry69 <starry369126@outlook.com> Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

)

…am-bot#2619, python-telegram-bot#2636)

…m-bot#2634) Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

python-telegram-bot#2657)

Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>

Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com> Co-authored-by: poolitzer <25934244+Poolitzer@users.noreply.github.com>

)

…-bot#2646)

Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

…elegram-bot#2724)

…am-bot#2717)

nivramam · 2021-10-28T03:54:24Z

Well, yes, making these changes everywhere where necessary is what we want. In addition

Please also make sure to add versioning directives and we should also add some tests for the changed attributes to make sure that they're None and not []

Sure, will do. Due to my exams I wasn't online here for 2 days. Will do so and give an update. Thx!

nivramam · 2021-10-29T06:25:35Z

Hi, so I fetched the latest from branch v14 yesterday. Later yesterday I noticed 1 check being canceled, which I am not aware of. And mainly I haven't changed/modified anything for it to fail. I just fetched upstream all the changes that were implemented in the past week. Have been trying to learn where the check has been canceled.

Surfed the same issue on the internet and people tend to say it's sometimes failing for some reason and not due to our PR. Anyway, I just thought of asking you guys. thanks in advance :)

Note: Attaching the screenshot of what is displayed for me in this page
Edit: Along with that, it also says there's no conflicts

Poolitzer · 2021-10-29T07:55:04Z

Don't worry about the checks until you implemented all the changes and then the feedback of the maintainers :D

nivramam · 2021-10-29T08:57:48Z

Okay, thanks!

nivramam · 2021-11-15T03:52:08Z

Hi, having a qq, while I was resolving a similar [] empty list issue in replykeyboardmarkup. Have described the same over here #2606 (comment)

test_message: test for updated change reg issue python-telegram-bot#2606 reply_keyboard_markup : ensure self.keyboard is not null before assigning to attributes

nivramam · 2021-11-18T16:25:31Z

Hi so I have added tests for the already implemented changes (test_message.py). Apart from that, the above mentioned addition of a null check on keyboardmarkup too I have committed to review the checks. Thx! And also I am in the process of reviewing other similar changes. Will update here as and when I complete them.

Bibo-Joshi

Hi. Sorry for the late reply. I addressed your question on ReplyKeyboardMarkup below.
I'll do a more thorough review once you indicate that you've made all required changes :)

Bibo-Joshi · 2021-11-19T17:56:55Z

telegram/_message.py

@@ -468,7 +468,7 @@ def __init__(
        self.audio = audio
        self.game = game
        self.document = document
-        self.photo = photo or []
+        self.photo = photo or None


the or None is redundant :)

Right yes, shall modify it :)

Bibo-Joshi · 2021-11-19T18:05:36Z

telegram/_replykeyboardmarkup.py

+        # ensure keyboard is notnull before assigning
+        if self.keyboard:
+            self._id_attrs = (self.keyboard,)


I don't think this check is necessary. keyboard is a mandatory argument and if the user passes something that's not a list of lists of buttons, that's their fault.
Additionally, the sanity check in line 95 alread checks the "list of lists" part. So if we get here, self.keyboard is [[]] in the worst case, which will pass the if self.keyboard check.

Right. Yeah the check at 95 was there, but I thought for ensuring the null check would be necessary.

But thanks to your comment, I just reviewed and realized it doesn't get to null anywhere. Sure will do changes and let you know

And regarding conflicts there is a huge number of conflicts showing up now but wasn't there when I pushed yesterday.. Is it for me alone?

just pull from v14 into this branch, it should be fine :)

Yes, I have tried to do it. I was prompted to create a PR and I did so, but now I am stuck here. Could you pls give me a solution so that I have the latest V14? Thanks in advance!

P.S: Due to my access issue I was unable to login to Github and hence the delay.

Here's the screen of the PR that I created when prompted.

Also in the current PR thread, I am notified that there are conflicts that cannot be automatically resolved..

Bibo-Joshi · 2021-12-27T20:27:06Z

Hi. We discussed this issue in the dev chat again and had a bit of a change of heart. While the main argument to change to None was to be uniform with the other attributes, the benfits of [] are compelling and the issue of uniformness can be largely circumvented by properly adapting the documentation. E.g. the docstring for Message.photo currently reads

photo (List[:class:telegram.PhotoSize]): Optional. Available sizes of the photo.

and should be changed to something like

photo (List[:class:telegram.PhotoSize]): Available sizes of the photo. This list is empty if the message doesn't contain a photo.

@nivramam I'm sorry that we reverted our decision only after you started to work on this PR. I hope that you haven't put too much time into it and that you can understand our decision. If your are interested, you're very welcome to instead PR for the mentioned updates of the documentation. In this case, please fetch the current doc-fixes branch and create a new branch based on that to add your changes to.

nivramam · 2021-12-29T14:35:53Z

Hi. We discussed this issue in the dev chat again and had a bit of a change of heart. While the main argument to change to None was to be uniform with the other attributes, the benfits of [] are compelling and the issue of uniformness can be largely circumvented by properly adapting the documentation. E.g. the docstring for Message.photo currently reads

photo (List[:class:telegram.PhotoSize]): Optional. Available sizes of the photo.

and should be changed to something like

photo (List[:class:telegram.PhotoSize]): Available sizes of the photo. This list is empty if the message doesn't contain a photo.

@nivramam I'm sorry that we reverted our decision only after you started to work on this PR. I hope that you haven't put too much time into it and that you can understand our decision. If your are interested, you're very welcome to instead PR for the mentioned updates of the documentation. In this case, please fetch the current doc-fixes branch and create a new branch based on that to add your changes to.

Oh fine. Yes no issues on my side since I didn't have access to GitHub in the last month so no more updates from my side.

Sure will try to PR the mentioned updates. Thanks!

Bibo-Joshi and others added 30 commits October 3, 2021 20:12

Temporarily enable tests for the v14 branch

5948095

Move and Rename TelegramDecryptionError to telegram.error.PassportDec…

53aca25

…ryptionError (python-telegram-bot#2621)

Add Code Comment Guidelines to Contribution Guide (python-telegram-bo…

af2516b

…t#2612) Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>

Improve Type Hinting for CallbackContext (python-telegram-bot#2587)

b5723f0

Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

Add Custom pytest Marker to Ease Development (python-telegram-bot#2628)

6ca92ec

Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>

Make BasePersistence Methods Abstract (python-telegram-bot#2624)

efea686

Signed-off-by: starry69 <starry369126@outlook.com> Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

Refactor Initialization of Persistence Classes (python-telegram-bot#2604

104eb17

)

Remove __dict__ from __slots__ and drop Python 3.6 (python-telegr…

ab262ae

…am-bot#2619, python-telegram-bot#2636)

Remove day_is_strict argument of JobQueue.run_monthly (python-telegra…

8b7cbcc

…m-bot#2634) Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

Drop Non-CallbackContext API (python-telegram-bot#2617)

5630f98

Fix Signatures and Improve test_official (python-telegram-bot#2643)

09043a4

Remove Deprecated Functionality (python-telegram-bot#2644)

d7a286c

Add User Friendly Type Check For Init Of {Inline, Reply}KeyboardMarkup (

d652a86

python-telegram-bot#2657)

Refine Dispatcher.dispatch_error (python-telegram-bot#2660)

bd1e28c

Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>

Warnings Overhaul (python-telegram-bot#2662)

a6673c8

Clear Up Import Policy (python-telegram-bot#2671)

b1017ef

Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>

Make InlineQuery.answer Raise ValueError (python-telegram-bot#2675)

bf62537

Doc Fixes (python-telegram-bot#2597)

2b4ab57

Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com> Co-authored-by: poolitzer <25934244+Poolitzer@users.noreply.github.com>

Move Defaults to telegram.ext (python-telegram-bot#2648)

b630e1b

Update Notification Workflows (python-telegram-bot#2695)

b0385c1

Pass Failing Jobs to Error Handlers (python-telegram-bot#2692)

681393f

Improve Signature Inspection for Bot Methods (python-telegram-bot#2686)

2c44dc0

Handle Filepaths via the Pathlib Module (python-telegram-bot#2688)

6965ae9

Add Filters.update.edited (python-telegram-bot#2705)

13433e3

Use Error Messages for Pylint Instead of Codes (python-telegram-bot#2700

01be85f

)

Introduce Builder Pattern for Updater and Dispatcher (python-telegram…

7ba5b3a

…-bot#2646)

Mark Internal Modules As Private (python-telegram-bot#2687)

eec90a0

Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

Accept File Paths for Updater/DispatcherBuilder.private_key (python-t…

163d598

…elegram-bot#2724)

Make Tests Agnostic of the CWD (python-telegram-bot#2727)

1adc96c

Refactor MRO of InputMedia* and Some File-Like Classes (python-telegr…

38a6a6d

…am-bot#2717)

harshil21 added the bug 🐛 label Oct 25, 2021

harshil21 and others added 2 commits October 26, 2021 18:51

Update Exceptions for Immutable Attributes (python-telegram-bot#2749)

36d09df

Merge branch 'python-telegram-bot:v14' into v14

7eecaca

Update Code Quality Dependencies (python-telegram-bot#2748)

c81a087

harshil21 added this to the v14 milestone Nov 4, 2021

Poolitzer and others added 2 commits November 6, 2021 15:19

Refactor Warnings in ConversationHandler (python-telegram-bot#2755)

3e8d1fa

Merge branch 'python-telegram-bot:v14' into v14

bdf0dcb

nivramam added 2 commits November 18, 2021 20:53

Issue python-telegram-bot#2606 - update tests

84636cf

test_message: test for updated change reg issue python-telegram-bot#2606 reply_keyboard_markup : ensure self.keyboard is not null before assigning to attributes

Update test_message.py

6a774bf

Bibo-Joshi reviewed Nov 19, 2021

View reviewed changes

Bibo-Joshi force-pushed the v14 branch from 3e8d1fa to 8b5966d Compare November 19, 2021 18:47

harshil21 force-pushed the v14 branch from abc75b1 to 0af4def Compare November 20, 2021 16:38

Bibo-Joshi force-pushed the v14 branch 3 times, most recently from 5c26c91 to 74c4270 Compare December 13, 2021 18:14

Poolitzer force-pushed the v14 branch from 74c4270 to 65f3599 Compare December 18, 2021 09:52

Bibo-Joshi closed this Dec 27, 2021

This was referenced Dec 27, 2021

[BUG] Message.photo is set to an empty list when not provided #2606

Closed

Make documentation more accessible #2633

Closed

github-actions bot locked and limited conversation to collaborators Jan 5, 2022

Bibo-Joshi added 🔌 bug pr description: bug and removed bug 🐛 labels Nov 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update _message.py - issue #2606 #2741

Update _message.py - issue #2606 #2741

nivramam commented Oct 21, 2021 •

edited

Loading

nivramam commented Oct 28, 2021

nivramam commented Oct 29, 2021 •

edited

Loading

Poolitzer commented Oct 29, 2021

nivramam commented Oct 29, 2021

nivramam commented Nov 15, 2021

nivramam commented Nov 18, 2021

Bibo-Joshi left a comment

Bibo-Joshi Nov 19, 2021

nivramam Nov 20, 2021

Bibo-Joshi Nov 19, 2021

nivramam Nov 20, 2021

nivramam Nov 20, 2021

harshil21 Nov 26, 2021

nivramam Dec 27, 2021 •

edited

Loading

Bibo-Joshi commented Dec 27, 2021

nivramam commented Dec 29, 2021

Update _message.py - issue #2606 #2741

Update _message.py - issue #2606 #2741

Conversation

nivramam commented Oct 21, 2021 • edited Loading

Checklist for PR

nivramam commented Oct 28, 2021

nivramam commented Oct 29, 2021 • edited Loading

Poolitzer commented Oct 29, 2021

nivramam commented Oct 29, 2021

nivramam commented Nov 15, 2021

nivramam commented Nov 18, 2021

Bibo-Joshi left a comment

Choose a reason for hiding this comment

Bibo-Joshi Nov 19, 2021

Choose a reason for hiding this comment

nivramam Nov 20, 2021

Choose a reason for hiding this comment

Bibo-Joshi Nov 19, 2021

Choose a reason for hiding this comment

nivramam Nov 20, 2021

Choose a reason for hiding this comment

nivramam Nov 20, 2021

Choose a reason for hiding this comment

harshil21 Nov 26, 2021

Choose a reason for hiding this comment

nivramam Dec 27, 2021 • edited Loading

Choose a reason for hiding this comment

Bibo-Joshi commented Dec 27, 2021

nivramam commented Dec 29, 2021

nivramam commented Oct 21, 2021 •

edited

Loading

nivramam commented Oct 29, 2021 •

edited

Loading

nivramam Dec 27, 2021 •

edited

Loading