Skip to content

Add Discord timestamp converter and transformer#10388

Open
AbstractUmbra wants to merge 2 commits intoRapptz:masterfrom
AbstractUmbra:feat/timestamp-converter
Open

Add Discord timestamp converter and transformer#10388
AbstractUmbra wants to merge 2 commits intoRapptz:masterfrom
AbstractUmbra:feat/timestamp-converter

Conversation

@AbstractUmbra
Copy link
Contributor

Summary

This PR adds both a converter and transformer for use with Discord's timestamp functionality. This is made easier for the end user with the addition of the @time utility in the client, and as such I felt the need to support this.

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

Copy link
Contributor Author

@AbstractUmbra AbstractUmbra left a comment

Choose a reason for hiding this comment

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

Some of my own thoughts on it.

raise BadBoolArgument(lowered)


_TIMESTAMP_PATTERN: re.Pattern[str] = re.compile(r'<t:(\d+):[tTdDfFsSR]>')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not keen on re-creating the same constant for both transformer and converter, but I wasn't sure where else I could put this to import from outside of utils.

self.argument: str = argument
super().__init__(f'{argument} is not a recognised boolean option')

class BadDatetimeArgument(BadArgument):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As this uses the built-in datetime.datetime class, I opted for naming all resources DatetimeX, even thought this is more for timestamps than datetimes.
Feels unclean to me but naming this eludes me.

@AbstractUmbra AbstractUmbra force-pushed the feat/timestamp-converter branch 2 times, most recently from 444cfba to 0bc6c97 Compare January 19, 2026 14:23
@vmphase
Copy link
Contributor

vmphase commented Jan 19, 2026

A few suggestions for the implementation:

  • The current pattern requires a style flag (e.g., :R), but, iirc, discord timestamps are valid without one (defaulting to :f). Probably, change the pattern to r'<t:(\d+)(?::[tTdDfFsSR])?>' to support both.
  • fromtimestamp(n).replace(tzinfo=...) creates a naive local time before converting. It's safer to use datetime.fromtimestamp(int(match[1]), tz=datetime.timezone.utc) directly.

I’d also consider using __slots__ = ('argument',) in BadDatetimeArgument to reduce memory overhead.

@AbstractUmbra AbstractUmbra force-pushed the feat/timestamp-converter branch from 9aee944 to 472f5aa Compare January 19, 2026 16:09
@AbstractUmbra
Copy link
Contributor Author

A few suggestions for the implementation:

  • The current pattern requires a style flag (e.g., :R), but, iirc, discord timestamps are valid without one (defaulting to :f). Probably, change the pattern to r'<t:(\d+)(?::[tTdDfFsSR])?>' to support both.
  • fromtimestamp(n).replace(tzinfo=...) creates a naive local time before converting. It's safer to use datetime.fromtimestamp(int(match[1]), tz=datetime.timezone.utc) directly.

I’d also consider using __slots__ = ('argument',) in BadDatetimeArgument to reduce memory overhead.

Good calls on those, thank you! 'twas an oversight on my part.

None of the other error classes implement said slots however, so I'll opt out of adding those for now.

@AbstractUmbra AbstractUmbra force-pushed the feat/timestamp-converter branch from 472f5aa to 363e3b7 Compare January 22, 2026 06:32


def _convert_from_timestamp(argument: str) -> datetime.datetime:
match = _TIMESTAMP_PATTERN.match(argument)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additionally I chose .match() over .search() since we'd expect to only parse the <t:xxxxxx:x> input only rather than search an arbitrary string.

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.

4 participants