Make (most) objects comparable

[Bibo-Joshi]

Closes #1708

Added _id_attrs to most of the telegram objects (more or less) by the following paradigm:

1. If object has some sort of ID, just use that
2. If object has only optional args, use all of them
3. If object has required arg, use that
4. If object hast required arg and exactly one of the optionals must be given, use all

Also added the corresponding tests.

On the way I encountered some difficulties:

1. Objects with nested lists as attributes, which are not hashable: I had to overwrite __hash__ to use a tuple representation of the lists, e.g. InlineKeyboardMarkup
2. ReplyKeyboardMarkup accepts both strings and KeyboardButtons while converting stings to KB only on to_dict. So I overwrote __equal__ to cope with this
3. There was no test for WebhookInfo. I added one
4. I couldn't come up with a clever way to test InputFile.input_media_content for equality (in case an actual file handler is passed, that is), so I left that and InputMedia… out
5. I have no clue of the passport module. All objects that have an ID already had the _id_attrs set and it seemed to me that all the other classes have no stand alone tests. So I left them out as well …

Needs to be updated, when

• API 4.5 API 4.5 #1508
• API 4.6 4.6 API update #1723
• API 4.7 API 4.7 #1858
• API 4.8

are merged

[Bibo-Joshi]

equality test for InlineKeyboardMarkup fails on Py2.7. Don't know why, but don't really care 🤷‍♂

[Poolitzer]

thanks for asking me to review, but I gonna decline, this is too much for me :D

[Bibo-Joshi]

test_set_game_score_1 is the one again …

[Bibo-Joshi]

Just a thought: should we make TelegramObject.__eq__ raise an error or warning, if it's comparing two objects, one of which has an emtpy _id_attrs tuple, i.e. it's not comparable?

Edit: I just went for it (see commit below)

[tsnoam]

As far as I can tell looks fine.
One question, though.

[Bibo-Joshi]

Went through the _id_attrs it set again and actually found some discussion points. Also I was thinking about if/how this behaviour should be documented. I see different possibilities here:

• Don't document. It's a detail for advanced users, who can read code. Also we don't want to make promises
• Make a note on TelegramObject along the lines "If the object has an ID, that's the one. If not, it's mostly the required args + meaningful optionals. We can't make promises. Check the code"
• Make a note on every object, which attrs are checked.

[Bibo-Joshi]

• Added documentation, also for the classes, who already had id attrs.
• Addressed some of the above mentioned stuff
• In the process, I added message.chat to the id attrs, as message_id is not unique across chats. Strictly speaking this is somewhat breaking. Then again, the behaviour wasn't documented yet, so I'd sleep well with that …
• Coday complains: Access to a protected member _id_attrs of a client class: if other._id_attrs == (): That's okay imo.
• py3.6 fails on set_game_score …

[Poolitzer]

Looks good to me.

[Poolitzer]

Sorry, I forgot the actual issue I have with this: Documentation. I like that you wrote down what gets compared where, but I dont like that being before the attributes of the object. I feel like most users will want to read about attributes rather then what gets compared if they compare this object, which means they have to read over this all the time when they open the docs. What about putting the what-gets-compared paragraph at the end, after the parameters?

[Bibo-Joshi]

What about putting the what-gets-compared paragraph at the end, after the parameters?

I fear that it's heard to find it there, especially in the docs of classes with many attributes. And it's only 1-3 lines, so it doesn't take up too much space, imo … The alternative would be to have an explicit docstring for __eq__(), but I don't think that I like that …

[Bibo-Joshi]

Some concerns about this PR were voiced offline. I'll try to summarize in hopes of not missing the point:

• Implementing __eq__() to compare only a subset of a classes attributes will be unintuitive for users
• This is especially true for TG objects that can change, e.g. two Message objects will be equal when testing for message_id only, while maybe having different text
• Custom comparison by attributes in easy to achive, so there is no reason to implement __eq__()

I'd like to add that this argumentation not only sees harm in this PR but also in the current use of _id_attrs

While I don't agree with this argumentation, I find the arguments important and like to have this properly discussed before going for merge. For completeness sake, here is my line of thought:

• We already have __eq__() implemented for objects with some kind of ID. So we could a) remove that capability, b) leave it as is, or c) extend it to all/most objects
• The default implementation of __eq__() is useless - I'd rather use is, if I want to compare by object identity. So giving objects a meaningful comparison is crucial imo, which is why I went for c)
• Currently this PR aims to set _id_attrs to the attrs that uniquely identify an object, i.e. this comparison is the "correct" one in most use cases
• This PR adds documentation, so == is no black box and if users need a different comparison, they can still implement that

[Bibo-Joshi]

We discussed the PR in the dev chat again and decided to merge.
I went over the changes once again ad made some minor corrections.
py3.8 fail unrelated, codacy has v13 issues. Merging.