Fix tick delta type handling
#546
Conversation
## `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'`).
freezegun/api.py
Outdated
| def tick(self, delta: Union[datetime.timedelta, int]=datetime.timedelta(seconds=1)) -> datetime.datetime: | ||
| if isinstance(delta, numbers.Real): | ||
| def tick(self, delta: Union[datetime.timedelta, float]=datetime.timedelta(seconds=1)) -> datetime.datetime: | ||
| if isinstance(delta, float): |
There was a problem hiding this comment.
It looks like this change causes the tests to fail.
If passing fractions causes this bit to fail, I think we should tackle that separately - this PR can just focus on the type fix.
There was a problem hiding this comment.
Sorry, my bad, it should be isinstance(delta, (int, float)). I got confused by mypy treating int as a subtype of float, but isinstance (correctly) doesn't.
As you suggested, I will split this off into another PR though. I theory, this could break people that managed to pass in some type that isinstance of numbers.Real and works with datetime.timedelta(secods=x). Though, I couldn't come up with any such type other than int and float.
|
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. |
There was a problem hiding this comment.
Looks great, thank you for fixing this and for adding the tests @robsdedude!
FrozenDateTimeFactory.tickThe
deltaargument is capable of handlingfloats. In previous versions of freezgun, the.pyitype annotations were correctly reflecting that. For some reason, when moving the type annotations into the.pyfile, this information got lost.Further, checking for
isinstance(delta, numbers.Real)is probably not what was intended asfraction.Fractionis a subclass ofReal, but will cause an error when passed intodatetime.timedelta(seconds=delta).StepTickTimeFactory.tickThe same issue with the type hint applies here.
Fruther, passing an integer/float
deltawould lead to that number being added to the frozendatetime.datetime, which is not a valid operation (TypeError: unsupported operand type(s) for +: 'datetime.datetime' and 'int').