i actually encountered similar issue in the branch i am currently working on: because media object share many common attributes, but without any real hierarchy, i created mixin classes for attributes, and incorporated them into the media classes. but then of course i cannot leave __slots__ empty if i ever want to ```to_dict``, but i can neither populate it due to mro conflicts that arise upon multiple inheritance.
So it kinda brings me back to creating boilerplate attributes in each of those media classes.

TBH I can't recall discussing mixins in #2573. Is this something additional that you experimented with? If you want to discuss it further, we should probably do that on #2573 for now :)

Making TelegramObjects immutable. I'm really not sure if we want that, but it's at least an interesting thought given that incoming updates represent a static information and changing them doesn't make sense at all.

Hmm, I can see someone writing custom logic to add attributes to Updates. Ofc they could/maybe they should write it for context, but if they have an old project, making the objects immutable will break their flow. We could maybe make a channel post about that?

And btw, if someone uses our library just for the objects and does something with them, would we break their code as well? Differently speaking, can you change the objects easily by e.g. inheriting from them, or before you innit those? If not, I would very much vote for asking the community if making TelegramObjects breaks their flow.

True, it would be a major breaking change. IMO we should at least discuss it for v14, because if we decide we would like this (as maintainers, or after asking the community - nice idea btw!), and we don't put it in v14, it will be a long while until it happens :)

sure. maybe put it after/before async if that makes a change for you?

Don't think it will make a big difference …

Another thought came up in the user group: When TG adds new attributes to a class, those are currently not accessible in PTB until we make a new release. one could handle that similarly to the api_kwargs argument of the bot methods, i.e. gather all arguments that we don't handle individually in an attribute TelegramObject.api_kwargs . that should easily work with both the **kwargs approach or the dataclass + inspect approach

PS: One would need to think about whether or not the api_kwargs should be included into to_json. maybe one could even add a parameter for that to to_json …

PPS: Updated MWE with api_kwargs, also comparing the runtime with pydantic - showing that the inspect approach isn't so bad after all :D

import inspect
import timeit

from pydantic import Extra, BaseModel


class InspectDeJson:
    _init_params = None

    def __init__(self, arg1, arg2, arg3, arg4, arg5, arg6, arg7, api_kwargs=None):
        self.arg1 = arg1
        self.arg2 = arg2
        self.arg3 = arg3
        self.arg4 = arg4
        self.arg5 = arg5
        self.arg6 = arg6
        self.arg7 = arg7
        self.api_kwargs = api_kwargs

    @classmethod
    def de_json(cls, data):
        # try-except is significantly faster in case we already have a correct argument set
        try:
            return cls(**data)
        except TypeError as exc:
            if str(exc).startswith('__init__() got an unexpected keyword argument'):
                if cls._init_params is None:
                    signature = inspect.signature(cls)
                    cls._init_params = signature.parameters.keys()

                allowed, api_kwargs = {}, {}
                for param, value in data.items():
                    if param in cls._init_params:
                        allowed[param] = value
                    else:
                        api_kwargs[param] = value
                return cls(**allowed, api_kwargs=api_kwargs)
            else:
                raise exc


class PydanticDeJson(BaseModel):
    arg1: str
    arg2: str
    arg3: str
    arg4: str
    arg5: str
    arg6: str
    arg7: str

    class Config:
        extra = Extra.allow

    @classmethod
    def de_json(cls, data):
        return cls(**data)


class KwargsDeJson:

    def __init__(self, arg1, arg2, arg3, arg4, arg5, arg6, arg7, **kwargs):
        self.arg1 = arg1
        self.arg2 = arg2
        self.arg3 = arg3
        self.arg4 = arg4
        self.arg5 = arg5
        self.arg6 = arg6
        self.arg7 = arg7

    @classmethod
    def de_json(cls, data):
        return cls(**data)


correct_data = {
        'arg1': 'arg1',
        'arg2': 'arg2',
        'arg3': 'arg3',
        'arg4': 'arg4',
        'arg5': 'arg5',
        'arg6': 'arg6',
        'arg7': 'arg7',
}
overfull_data = correct_data.copy()
overfull_data.update({str(i): i for i in range(5)})

number = int(5*10e3)

print('Passing correct data')
a = timeit.timeit('InspectDeJson.de_json(correct_data)', globals=globals(), number=number)
b = timeit.timeit('KwargsDeJson.de_json(correct_data)', globals=globals(), number=number)
c = timeit.timeit('PydanticDeJson.de_json(correct_data)', globals=globals(), number=number)
print(f'InspectDeJson took {a/number} seconds on average')
print(f'KwargsDeJson took {b/number} seconds on average')
print(f'PydanticDeJson took {c/number} seconds on average')
print(f'InspectDeJson took {a*100/b} % of KwargsDeJsons runtime')
print(f'PydanticDeJson took {c*100/b} % of KwargsDeJsons runtime')

print('\n' + 20*"=" + '\n')


print('Passing too much data')
a = timeit.timeit('InspectDeJson.de_json(overfull_data)', globals=globals(), number=number)
b = timeit.timeit('KwargsDeJson.de_json(overfull_data)', globals=globals(), number=number)
c = timeit.timeit('PydanticDeJson.de_json(overfull_data)', globals=globals(), number=number)
print(f'InspectDeJson took {a/number} seconds on average')
print(f'KwargsDeJson took {b/number} seconds on average')
print(f'PydanticDeJson took {c/number} seconds on average')
print(f'InspectDeJson took {a*100/b} % of KwargsDeJsons runtime')
print(f'PydanticDeJson took {c*100/b} % of KwargsDeJsons runtime')

Passing correct data
InspectDeJson took 1.635704e-06 seconds on average
KwargsDeJson took 1.9409600000000003e-06 seconds on average
PydanticDeJson took 3.8132918e-05 seconds on average
InspectDeJson took 84.27293710328908 % of KwargsDeJsons runtime
PydanticDeJson took 1964.6421358503005 % of KwargsDeJsons runtime

====================

Passing too much data
InspectDeJson took 9.215419999999996e-06 seconds on average
KwargsDeJson took 3.0822979999999946e-06 seconds on average
PydanticDeJson took 3.994753999999999e-05 seconds on average
InspectDeJson took 298.97887874566356 % of KwargsDeJsons runtime
PydanticDeJson took 1296.0310781112034 % of KwargsDeJsons runtime

@Bibo-Joshi That's a nice idea but we should also mind the pitfall:
Let's say you persist an object with these api_kwargs.
Now let's say you upgrade ptb, restart the bot and try to load from persistence the persisted object.
How would you like loading from persistence to behave?

How would you like loading from persistence to behave?

Some thoughts that we had on this in the dev chat:

1. on __init__, we can do something like

   def __init__(self, arg, api_kwargs):
       if arg is None:
           arg = api_kwargs.get('arg', None)

   This assigns the additional api kwargs to the proper attributes while still having them in api_kwargs for compatibility

2. We can make sure that the pickle behavior of TelegramObject does something similar (either by explicitly calling __init__ or by other means)

3. We can't cover all persistence implementations, b/c not every serialization method uses pickle or calls __init__.

Some more thoughts about immutability:

• if you inherit from a dataclass(frozen=True), there are two cases
  • if the child class is a dataclass as well, it must have frozen=True as well
  • if the child class is no dataclass, added attributes are not frozen. inherited attributes are frozen.
• Nothing is really immutable in python. object.__setattr__(immutable_obj, 'attr', value) will set immutable_obj.attr to value.
• if a class implements __hash__, two objects should have the same hash value if and only if they are equal in terms of __eq__. Moreover, the hash value of on object should not change. This also implies that mutable objects should not be hashable.
  • attributes of TelegramObject & subclasses are currently mutable, i.e. users can change them. These changes are reflected neither in __eq__ nor in __hash__.
  • We even break the "hash value should not change" rule with InlineKeyboardButton.update_callback_data. (Although this is probably only a minor violation as the button is exposed to the user only after the one-time change of the hash value)
  • We currently rely on hashability of TelegramObject e.g. in Message.parse_entities which returns a dict with MessageEntity objects as keys
  • We use lists as pythons default equivalent of json arrays. lists are mutable - in contrast to tuples. If we would like to achieve immutability, we should consider switching to tuples.
• Immutability has two different aspects: For classes like Messages, which a user will never create, it makes sense, IMO. For classes like BotCommandScope*, which are only created by the user and then passed to TG, it's easier to think of use cases, where mutability may be a small advantage.

Also just wanted to say since py 3.11, there is zero cost exception handling, so another plus point for the inspect approach.

And note that there is apparently a small performance penalty when using frozen instances. Can we benchmark this and see how much it really is for our case?

Yesterday I tried to get started on a proof of concept. And I almost immediately ran into two major problems, which frankly I'm embarrased to not have seen so far. Both are related to optional arguments.

Slots

Not only do dataclasses bring built-in support for slots only on python 3.10+, but it's actually not possible to directly use them manually on 3.9- with default arguments:

@dataclass
class Test:
    __slots__ = ("arg",)

    arg: str = "arg"

will raise an exception. Actually this is not due to dataclasses directly (removing @dataclass still raises) but due to __slots__ inserting stuff into the class namespace. See e.g. https://stackoverflow.com/questions/50180735 for more input. We actually face this very problem currently in TelegramObject where we work around this with a custom __new__ method. Unfortunately, this can't help us with dataclasses.

Inheritance

The second problem appears when we want to inherit from a dataclass with optional arguments:

@dataclass
class Parent:
    arg: str = "arg"


@dataclass
class Child(Parent):
    barg: str

now dataclass will build an __init__ for Child which looks like __init__(self, arg: str = "arg", barg: str) - and that's not allowed.
Most of our classes inherit directly from TelegramObject, which doesn't have any arguments at all, so this might sound not so important. However, this becomes relevant when adding an optional argument api_kwargs to TO as mentioned a few comments up. A straightforward workaround is to shift this from TO into all the inheriting classes. Note nice, but does the trick. We face more severe problem when considering InputMedia, which does have optional arguments and multiple child classes. Moving the default arguments would mean to rob InputMedia of these arguments, which strictly speaking is more in line with the API (where InputMedia doesn't have any attributes at all) but still feels wrong to me. Note that this also affects internal helpers like _BaseMedium.
In python 3.10, one can mark dataclass arguments as "kw only" which somehow works around this problem because __init__(self, *, arg: str = "arg", barg: str) is a valid signature. IMO it still doesn't really address the issue.
I'm not sure why dataclasses don't just create an __init__ of the form __init__(self, pos_args_of_parent, pos_args_of_child, kw_args_of_parent, kw_args_of_child) but knowing the reasons probably wouldn't change much.

So far these are my observations and I'm rather bummed about that. I haven't gotten far in terms of thinking about our options, yet …

A few more insights:

pydantic

• uses kword-only signatures, which alleviates the problem with optional arguments just like dataclasses do in py3.10+
• neither __new__ nor __init__ have an explicit signature. inspact.signature still is able to derive a signutare for the class. IISC that's done by some rather deep python trickery.

attrs

• __init__ has an explicit signature
• this also means that one has the same problem with optional arguments again, which can only be resolved by making the arguments keyword-only

Big picture

This issue came to be because at least #2491 and #2607 looked like they could be resolved easily by using dataclasses and because they looked promising in terms of saving us some boilerplate code. But in the end, they apparently don't fit our requirements and using them just for the sake of it doesn't make sense. 3rd party libs also can't really help (see above) and we're no fans of extensive 3rd party use anyway.

So let's evaluate what our options are to implement the things that we were considering in the context of dataclasses, without dataclasses.

api_kwargs

This is basically independent of dataclasses. The ideas outlined in this thread can readily be applied in a boilerplate setting

__slots__

AFAIK slots need to be added at class creation. So if we'd want to automate this, we'd have to e.g. build a class decorator that returns a new, slotted class. I'll wager developing something like this adds nothing to the library except bugs and maintenance work.

type annotations

context: #3008, microsoft/pylance-release#1983, i.e. pylance expects us to do

def __init__(self, arg: str):
    self.arg: str = arg # not the ": str"

if we have to touch all attributes anyway for some reason, then IMO we could add this, otherwise I'm perfectly fine with keeping as is.

Immutability

Originally this was a "dataclasses comes with support for this, so we could consider doing this", but in the recent discussion, there was enough interest from the dev team for this direction to also consider it without dataclasses.
I guess there are a few options here. From a first glance I like the descriptor thingy best. But benchmarks and other comparisons are probably good and there are surely also other options.

@property

Deas simple, lots of boilerplate:

class Message:
    def __init__(self, message_id: int):
        self._message_id = message_id
    
    @property
    def message_id(self):
        return self._message_id

Descriptors

Properties are descriptors, so one should be able to cook up something like

Deas simple, lots of boilerplate:

class Message:
    def __init__(self, message_id: int):
        self.message_id= ReadOnlyDescriptor(message_id)

__setitem__

Something along the lines

class Message:
    def __init__(self, message_id: int):
        self.message_id = message_id
    
    def __setitem__(self, name, value):
        if not name.startswith('_'):
            raise RuntimeError
        # otherwise set attribute

This might have some side effects on the implementation that I don't see just yet … Would have to investigate more.

Oh, I forgot:

__str__ and __repr__

Recreating the full pprint functionality (indent, line cut-off etc) is probably out of scope, but doing a simple Message(message_id=1, from=User(…), …) should be doable, IMO.