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

[Poolitzer]

Steps to reproduce

1. Generate a Message object without photo set.

2. Check the photo parameter

Expected behaviour

.photo should be None

Actual behaviour

.photo is an empty list

Wrong line:

python-telegram-bot/telegram/message.py

Line 515 in 1fdaaac

self.photo = photo or []

[Poolitzer]

Technically breaking (I guess?) and we want to do V14 changes now anyway, so on that list it goes

[Bibo-Joshi]

@Poolitzer Similar for other attributes in Message and Game and we should check if there are other classes with this.

I see this was introduced in #788 - I don't know if by @jsmnbom or by @Eldinnie . Do you recall what you meant by

A few of the attributes of Game and Message now default to list() instead of None since they are indeed lists no matter what

? By this statement I would have expected to find photo: [] in the JSON message sent by TG, but that's not the case …
(As usual, just checking if we're missing some point 🙂)

Edit: We had some discussion in dev chat about this, see https://t.me/c/1494805131/17069 and answers

[nivramam]

Hi, is this issue still active?

[Bibo-Joshi]

Hi @nivramam . IIRC we haven't made a final decision on this, which is why I didn't tag it for hacktoberfest.

The reason why this was implemented in #788 was that e.g. for photo_size in message.photo will just always work if message.photo is an empty list and if message.photo works just as well. But I do see that it doesn't comply with the rest of the implementation.

Personally, I'd vote +0.5 on this, i.e. I'd be in on changing this but also wouldn't complain if it isn't changed.
@Poolitzer you're probablby +1 since you opened the issue? @harshil21 @starry69, what's your opinion?

[harshil21]

@Bibo-Joshi I think we should change it to None. I usually check my list before running a for loop on it.

[nivramam]

Yeah, maybe changing to None would be better

[Poolitzer]

I actually personally really like the argument that for photo_size in message.photo works, because it could lessen code complexity. But I dont think many devs will use that. So I will vote a 0.5 as well: Dont care if implemented or not, but am fine if someone wants to taggle it

[Bibo-Joshi]

@nivramam I think we have a rather clear vote, then with +2/3. If you want to, you're welcome to PR for this issue :)

[nivramam]

@Bibo-Joshi Sure, would do. I've just started with contributing and learning on PRs, so a bit scratchy. Would definitely approach if I get stuck in between. As for now,we need to work changes on v14 right?

[Poolitzer]

Yes, switch to the v14 branch on your repo, and then base your working branch off of that one

[nivramam]

Hi, I performed the change (self.photo=None) in the telegram/message.py file and tested the same. The following is the output I got. Since I have just started out, I thought I can confirm here whether my track is right. While testing the change, the following "deprecated" message is coming, can anyone guide me as to how I need to check that?

Thanks in advance

[Bibo-Joshi]

This means that you're adding attributes to the Message object that are not defined in __slots__. However, _id_attrs certainly are defined in __slots__, so I'm not sure what you changed …
please be sure to base your branch on the v14 branch as described in #2681. on that branch you'll already get an exception rather than a warning :)

[nivramam]

OKay, let me check it then. Thanks

[nivramam]

Hey there. I am not modifying any _id_attrs or _slots_ in my code snippet, just trying to make sure that Message.photo displays None - just that single line modification https://github.com/python-telegram-bot/python-telegram-bot/blob/1fdaaac8094c9d76c34c8c8e8c9add16080e75e7/telegram/message.py#L515

I also checked if i have the latest version. I fetched upstream again and tried testing that change, which again gives this deprecated warning. What shall I do, can I proceed to create the PR?

[Bibo-Joshi]

In the screenshot above it looks like you're working with some custom files. Maybe that's the problem. Make sure to edit the file telegram/_message.py. If you haven't already, please be sure to check out the contribution guide :)

[nivramam]

Yeah, I have edited the telegram/message.py file

[nivramam]

Yes I went through the guide, but getting doubts as I do, sorry, that's why I'm approaching here..

[Bibo-Joshi]

Okay, then I don't understand what's going on and would need to see code. So, yeah, you can go ahead and PR and we'll have a look at what's going on :)

[nivramam]

Sure, thanks.

[nivramam]

So while I was just reviewing for other changes required on similar lines to the above, in _replykeyboardmarkup.py file, I understand we try to append the button rows (provided as parameter) into our class's data - self.keyboard. Hence we init it with empty list. But after the appending process is done via the for loop, I wondered if we needed a check to ensure the data has actually been appended and the self.keyboard is not equal to [] after the appending.

This is the snippet I am referring to..

Please do give in your inputs, would be helpful for me to understand better.. Thanks a lot

[Bibo-Joshi]

Closing for the reasons mentioned in #2741 (comment) and instead putting this on the todo list of #2633