Skip to content

Use timedelta to represent time periods in classes #4750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Use timedelta to represent time periods in classes #4750

wants to merge 4 commits into from

Conversation

aelkheir
Copy link
Member

@aelkheir aelkheir commented Apr 9, 2025

Second part of #4575.

Affected classes (basicaly anywhere "in seconds" appears in api docs)
Class attribute docs
- [x] ChatFullInfo slow_mode_delay Optional. For supergroups, the minimum allowed delay between consecutive messages sent by each unprivileged user; in seconds
-[x] ChatFullInfo message_auto_delete_time Optional. The time after which all messages sent to the chat will be automatically deleted; in seconds
Animation duration Duration of the video in seconds as defined by the sender
Audio duration Duration of the audio in seconds as defined by the sender
Video duration Duration of the video in seconds as defined by the sender
Video start_timestamp Optional. Timestamp in seconds from which the video will play in the message
VideoNote duration Duration of the video in seconds as defined by the sender
Voice duration Duration of the audio in seconds as defined by the sender
PaidMediaPreview duration Optional. Duration of the media in seconds as defined by the sender
Poll open_period Optional. Amount of time in seconds the poll will be active after creation
Location live_period Optional. Time relative to the message sending date, during which the location can be updated; in seconds. For active live locations only.
MessageAutoDeleteTimerChanged message_auto_delete_time New auto-delete time for messages in the chat; in seconds
VideoChatEnded duration Video chat duration in seconds
ChatInviteLink subscription_period Optional. The number of seconds the subscription will be active for before the next payment
ResponseParameters retry_after Optional. In case of exceeding flood control, the number of seconds left to wait before the request can be repeated
InputMediaVideo duration Optional. Video duration in seconds
InputMediaAnimation duration Optional. Animation duration in seconds
InputMediaAudio duration Optional. Duration of the audio in seconds
InputPaidMediaVideo duration Optional. Video duration in seconds
InlineQueryResultGif gif_duration Optional. Duration of the GIF in seconds
InlineQueryResultMpeg4Gif mpeg4_duration Optional. Video duration in seconds
InlineQueryResultVideo video_duration Optional. Video duration in seconds
InlineQueryResultAudio audio_duration Optional. Audio duration in seconds
InlineQueryResultVoice voice_duration Optional. Recording duration in seconds
InlineQueryResultLocation live_period Optional. Period in seconds during which the location can be updated, should be between 60 and 86400, or 0x7FFFFFFF for live locations that can be edited indefinitely.
InputLocationMessageContent live_period ptional. Period in seconds during which the location can be updated, should be between 60 and 86400, or 0x7FFFFFFF for live locations that can be edited indefinitely.

Check-list for PRs

  • Added .. versionadded:: NEXT.VERSION, .. versionchanged:: NEXT.VERSION, .. deprecated:: NEXT.VERSION or .. versionremoved:: NEXT.VERSION to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)
  • Created new or adapted existing unit tests
  • Documented code changes according to the CSI standard <https://standards.mousepawmedia.com/en/stable/csi.html>__
  • Checked the Stability Policy <https://docs.python-telegram-bot.org/stability_policy.html>_ in case of deprecations or changes to documented behavior

If the PR contains API changes (otherwise, you can ignore this passage)

  • Checked the Bot API specific sections of the Stability Policy <https://docs.python-telegram-bot.org/stability_policy.html>_

  • Created a PR to remove functionality deprecated in the previous Bot API release (see here <https://docs.python-telegram-bot.org/en/stable/stability_policy.html#case-2>_)

  • If relevant:

    • Added or updated documentation for the changed class(es) and/or method(s)

aelkheir added 3 commits April 9, 2025 02:51
@aelkheir
Setup helper functions and a common test fixture.
194f9de
@aelkheir
Adopt ChatFullInfo & tests.
5efa69f 
- [x] `ChatFullInfo.slow_mode_delay`.
- [x] `ChatFullInfo.message_auto_delete_time`.
@aelkheir @github-actions
Add chango fragment for PR #4750
f83e066
@aelkheir
Copy link
Member Author

aelkheir commented Apr 9, 2025 •
edited
Loading

One thought before i proceed with the rest of the classes.

Both attributes and arguments should only accept timedeltas in the long run. #4575 (comment)

why deprecate class args, i think it would be convenient to allow passing either 30 or timedelta(seconds=30). it would also be consistent with how bot methods work as of #4651. deprecating class attrs is sensible however.

@Bibo-Joshi
Copy link
Member

Hey, thanks for getting started so quickly:)
About your question: TelegramObject types are not really intended to be manually initialized by the user but rather by the de_json methods - that's in contrast to the bot methods. I view them as mere data storages. Note that the conversions from json to datetime or off of the TG classes also all happen in de_json and not in init. This approach can certainly be questioned in general (i.e. what would be a sensible interface for classes wrapping API types). In the current set-up, I'd see consistency as the more important argument 😅

@aelkheir
Copy link
Member Author

aelkheir commented Apr 9, 2025

Aha okay. but would it still be necessary to issue a separate deprecation warning in the init methods as was outlined in #4575 (comment) (very helpful summary btw 😄 ). one is already being issued on property access.

@Bibo-Joshi
Copy link
Member

I would say, so yes. Attribute access and passing the parameter are two independent operations, both of which will be affected :)

@aelkheir
Copy link
Member Author

aelkheir commented Apr 9, 2025

I see! I was thinking more from the perspective that

TelegramObject types are not really intended to be manually initialized by the user rather by the de_json.

whom are we intending to warn then

anyways I did include both warnings for completeness and the cases where users are manually initializing objects

@aelkheir aelkheir mentioned this pull request Apr 24, 2025
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aelkheir @Bibo-Joshi