Skip to content
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.

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

Fix tick delta type handling #546

Merged
merged 6 commits into from
May 8, 2024
Merged

Fix tick delta type handling #546

merged 6 commits into from
May 8, 2024

Conversation

robsdedude
Copy link
Contributor

FrozenDateTimeFactory.tick

The delta argument is capable of handling floats. In previous versions of freezgun, the .pyi type annotations were correctly reflecting that. For some reason, when moving the type annotations into the .py file, this information got lost.

Further, checking for isinstance(delta, numbers.Real) is probably not what was intended as fraction.Fraction is a subclass of Real, but will cause an error when passed into datetime.timedelta(seconds=delta).

StepTickTimeFactory.tick

The same issue with the type hint applies here.

Fruther, passing an integer/float delta would lead to that number being added to the frozen datetime.datetime, which is not a valid operation (TypeError: unsupported operand type(s) for +: 'datetime.datetime' and 'int').

## `FrozenDateTimeFactory.tick`
The `delta` argument is capable of handling `float`s. In previous versions of
freezgun, the `.pyi` type annotations were correctly reflecting that. For some
reason, when moving the type annotations into the `.py` file, this information
got lost.

Further, checking for `isinstance(delta, numbers.Real)` is probably not what
was intended as `fraction.Fraction` is a subclass of `Real`, but will cause an
error when passed into `datetime.timedelta(seconds=delta)`.

## `StepTickTimeFactory.tick`
The same issue with the type hint applies here.

Fruther, passing an integer/float `delta` would lead to that number being added
to the frozen `datetime.datetime`, which is not a valid operation
(`TypeError: unsupported operand type(s) for +: 'datetime.datetime' and 'int'`).
Copy link
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

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

Hi @robsdedude, thanks for the PR! Please see my comments

freezegun/api.py Outdated Show resolved Hide resolved
freezegun/api.py Outdated Show resolved Hide resolved
@robsdedude robsdedude marked this pull request as draft May 6, 2024 07:21
@robsdedude robsdedude marked this pull request as ready for review May 6, 2024 07:42
@robsdedude
Copy link
Contributor Author

I came up with what I think is a way to keep the runtime functionality as is, and get the type hints to be somewhat ™️ accurate.

Copy link
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

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

Looks great, thank you for fixing this and for adding the tests @robsdedude!

@bblommers bblommers merged commit ea054a3 into spulec:master May 8, 2024
15 checks passed
@robsdedude robsdedude deleted the patch-1 branch July 15, 2024 07:28
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.

None yet

2 participants