Pewarta Proxy

📋 Server Request & Proxy Logs

Loading active proxy settings...
Developed by Khairul Adha (adhexa.id)
Mengambil log server...

⚙️ Pengaturan Proxy Target

Skip to content

Switching to Dataclasses - #5334

Open
aelkheir wants to merge 32 commits into
masterfrom
dataclasses
Open

aelkheir wants to merge 32 commits into
masterfrom
dataclasses

Conversation

@aelkheir

@aelkheir aelkheir commented Aug 23, 2026 •

Copy link
Copy Markdown
Member

Closes #5279
Discussed Priorly: #2698

@aelkheir aelkheir changed the title Dataclasses WIP: Switching to Dataclasses Aug 23, 2026
Except `inputmedia.py` classes and `Bot`
That makes all TOs, other application classes still have their
tests
dataclasses generated `hash()` does not include `self.__class__`
so this is to preserve old behaviour of different classes, having
same field level hashes, not being equal
Note: this failure revealed a minor breaking change in how `__eq__()`
works, worth documenting and adding to changelog later
Old test for a now breaking change unfortunately.
I temporarily broke old TO.__hash__ behaviour that used identity hashing
when two objects had no _id_attrs (i.e empty tuple). Not sure why it was
that way since empty id attrs mean their equall in terms of __eq__ (yes
we did issue a warning but still), so they should have same hash.

Anyways that should be restored now because otherwis it would be a
breaking change.
 Conflicts:
	tests/_files/test_animation.py
	tests/_files/test_audio.py
	tests/_files/test_chatphoto.py
	tests/_files/test_document.py
	tests/_files/test_photo.py
	tests/_files/test_sticker.py
	tests/_files/test_video.py
	tests/_files/test_videonote.py
	tests/_files/test_videoquality.py
	tests/_files/test_voice.py
	tests/test_forum.py
Same breaking change caused by a bug in old `__eq__()`.
tl;dr previously __eq__ allowed parent == child # True
@aelkheir
aelkheir marked this pull request as ready for review September 23, 2026 17:29
@aelkheir

aelkheir commented Sep 23, 2026 •

Copy link
Copy Markdown
Member Author

So I had this drafted for a bit of time now, I intended to send it earlier on to get your feedback on it, especially the logic around the after-processing of generated __init__ and whether that path seems acceptable to you.

Then I was tempted to walk till (near) the finish line and see how (simple v.s complicated) it will turn to be.


Future Stability Policy handling
arg: InitVar and _arg = field(init=False)

This did not age well.

from dataclasses import InitVar, dataclass, field


@dataclass(frozen=True)
class Example:
    value: InitVar[int | None] = None
    _value: int | None = field(init=False, default=None)

    def __post_init__(self, value: int | None, /):
        object.__setattr__(self, "_value", value)

    @property
    def value(self) -> int | None:
        print("value is deprecated and will be removed")
        return self._value


a = Example()
print(a._value)  # output: <property object at 0x7ff25e629e40>

b = Example(10)
print(b._value)  # output: 10

When not providing an explicit argument for value, the default None is overwritten by the property object.
A relevant CI issue also is that, both linting (ruff) and type checking (mypy) report:
error: Name "value" already defined/Redefinition of unused \value``

So i tried giving this alias Field specifier parameters a shot:

alias is an optional str parameter that provides an alternative name for the field. This alternative name is used in the synthesized init method.

The outcome of it is pretty neat and less verbose, however, and just like converter we have to provide runtime impl

Aliased parameter MWE
# No runtime support fo alias, this is just to demonstrate type checkers support
from dataclasses import MISSING, dataclass, field
from typing import Any, TypeVar, dataclass_transform

_T = TypeVar("_T")


def tg_field(
    *,
    default: Any = MISSING,
    alias: str | None = None,
) -> Any:
    return field(default=default)


@dataclass_transform(field_specifiers=(tg_field,))
def tg_dataclass(cls: type[_T]) -> type[_T]:
    return dataclass(frozen=True)(cls)


@tg_dataclass
class Example:
    _value: int | None = tg_field(alias="value", default=None)

    @property
    def value(self) -> int | None:
        print("value is deprecated and will be removed")
        return self._value


# Wroks in runtime
c = Example(_value=10)

# TypeError: Example.__init__() got an unexpected keyword argument 'value'
d = Example(value=10)

Its well supported in type checkers that common IDEs use

IDE Support Pyright Neovim with Pyright

VSCode Pylance
image

Pycharm, should be supported from 2024.3 onwards. https://youtrack.jetbrains.com/issue/PY-73832/Pycharm-doesnt-properly-recognizes-aliases-for-Pydantic-dataclass
image

mypy also support aliases running it through the above MWE gives:
Unexpected keyword argument "_value" for "Example"; did you mean "value"? [call-arg]

I honestly don't know what to do about temp unfreezing ...

I just did nothing. Excluding test files, there was very minimal unfreezing of objects outside of old __init__s, so I just object.__seattr__ these. For tests, mutations use a combination of a new with _unfrozen(obj) helper and object.__seattr__

CI

Sadly pylint and mypy have a bunch of false positives that we'll have to fight some way.
pylint: pylint-dev/pylint#4899, pylint-dev/pylint#7437
mypy: python/mypy#17547


I still got some polishing to do, specifically I want to:

  • Remove the enum converters and simplify with functool.partial
  • Add a TO._set(). object.__setattr__ was still a one-liner but it takes too much horizontal space
  • Add deprecation notes to the changelog, mostly parameter order changes
  • Add tests for _utils/dataclass.py
  • Update Checklist for PRs
  • Adapt test_official as needed.
  • Experiment with no longer tampering with the generated __init__ signatures and annotations and instead make TO._de_json() dataclasses-first, meaning to look up the correct annotation from dataclasses.fields(obj) directly rather than inpecting __init__. (For later)
  • Apply similar treatment to test_official (For later)

The last two points can further simplify _utils/dataclass.py, however the current implementation of the helper is the most truthful about what __init__ actually accepts but also fights python and standard dataclasses behaviour.

I'm going to be working on these items in the following days, would be great to get some initial review since tests/ are passing now

Doesn't have to be the whole thing, just _utils/dataclass.py and the changelog.

@aelkheir aelkheir changed the title WIP: Switching to Dataclasses Switching to Dataclasses Sep 23, 2026

@harshil21 harshil21 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of reviews! Overall, I'm pretty happy with the implementation so far. I only looked at TGO.py and dataclasses.py in this round.

Experiment with no longer tampering with the generated init signatures and annotations and instead make TO._de_json() dataclasses-first,

I would probably leave this for a future PR (in the interest of time), I think we have right now can work fine.

One thing I would double check is if backward / forward compatibility isn't affected by this. I know we have tests, but can't hurt to check if they work with these changes..

Comment on lines +6 to +7
- Direct instances of :class:`~telegram.TelegramObject` are frozen by default.
- Loading a pickled :class:`~telegram.TelegramObject` will return a frozen instance by default.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was already existing behaviour?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True for subclasses
But if you initialize a TelegramObject directly it used to come unfrozen.
We had a comment there saying it was to make subclasses not have to call unfrozen in every single one of their inits

For pickling, previously unpickling a (unfrozen when pickled) object would load it unfozen just like it was. Now it will be loaded frozen regardless.

Thanks I'll now rephrase that second point i can see it can be unclear

"""Special marker indicating that an entry was deleted."""


def to_sequence_annotation(annotation: object) -> object:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a docstring showing why this is needed would be nice


from telegram._utils.de_json import unwrap_optional

_T = TypeVar("_T")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can use the Self type now that we're dropping 3.10 support

return cls

aliased_init.__signature__ = signature # type: ignore[attr-defined]
cls.__init__ = aliased_init # type: ignore[method-assign]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this means that during initialization, the for loop of aliased_init is going to run every time checking every field. If we replace has_aliases with:

alias_pairs = tuple(
    (dataclass_field.name, alias)
    for dataclass_field in fields.values()
    if dataclass_field.init
    and (alias := dataclass_field.metadata.get(ALIAS_KEY)) is not None
)

and then just do the for loop with the alias pairs then that should be slightly faster.

def tg_dataclass(
*,
eq: bool = True,
) -> Callable[[type[_T]], type[_T]]:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put a docstring for this and tg_field

Comment thread src/telegram/_utils/dataclass.py Resolved
Comment on lines +137 to +138
for dataclass_field in dataclasses.fields(self):
converter = dataclass_field.metadata.get(CONVERTER_KEY)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can do a similar thing here like my previous comment about aliases: cache the fields which need a converter and only iterate over those, instead of every field


if TYPE_CHECKING:
# FIXME: not sure what to do about this
# mypy doesn't recognize converters currently

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self: it's time to migrate to ty and drop mypy

@harshil21 harshil21 mentioned this pull request Oct 23, 2026
22 tasks

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switching to Dataclasses

2 participants