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

Ensure unix_time provider returns an integer #1950

Closed
wants to merge 1 commit into from

Conversation

karl-potisepp
Copy link

@karl-potisepp karl-potisepp commented Nov 14, 2023

Based on the common understanding that a UNIX timestamp is the number of seconds since 00:00:00 UTC on 1 January 1970, and also the typing information provided in the function signature, this change forces the returned timestamp to integer precision.

What does this change

This casts the result of unix_time into an integer.

What was wrong

unix_time provider was returning float values, which is in conflict with the specified return type of int. For reference, here is the Wikipedia page for UNIX time.

How this fixes it

This casts the result of unix_time into an integer.

Based on the common understanding that a UNIX timestamp is the number of seconds since 1970-01-01 and also the typing information provided in the function signature, this change forces the returned timestamp to integer precision.
@stefan6419846
Copy link
Contributor

I am not sure whether changing the type hint to float is the more appropriate solution here, as time.time does return a float as well: https://docs.python.org/3/library/time.html#time.time (The stubs state a float as well: https://github.com/python/typeshed/blob/main/stdlib/datetime.pyi)

@karl-potisepp
Copy link
Author

I am not sure whether changing the type hint to float is the more appropriate solution here, as time.time does return a float as well: https://docs.python.org/3/library/time.html#time.time (The stubs state a float as well: https://github.com/python/typeshed/blob/main/stdlib/datetime.pyi)

I agree that this is also a solution to the issue. Although I would say - from personal experience - that having the UNIX timestamp as an integer type is a very widely spread practice, and this can break a lot of users code (and lead them to come here and make pull requests like this :) ). I don't think I am wrong when I say that calling it unix_time creates an expectation of getting back a 32 bit integer.

In any case, I will defer to the decision of the maintainers. Thank you for your support!

@fcurella
Copy link
Collaborator

My preference is to go with what Python does and be consistent with the language.

@fcurella fcurella closed this Nov 14, 2023
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.

3 participants