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

Dealing with naive datetime objects #138

Closed
tilsche opened this issue Dec 14, 2022 · 1 comment
Closed

Dealing with naive datetime objects #138

tilsche opened this issue Dec 14, 2022 · 1 comment
Labels
discussion There are several possible ways to resolve this issue that need a discussion enhancement New feature or request

Comments

@tilsche
Copy link
Contributor

tilsche commented Dec 14, 2022

Passing a naive datetime object to Timestamp.from_datetime currently leads to:

    delta = dt - Timestamp._EPOCH
TypeError: can't subtract offset-naive and offset-aware datetimes

Now passing naive objects may be dangerous. And maybe we don't want this. That we need to discuss. I see the following options

  • Support both aware and naive datetime objects in from_datetime. In accordance with datetime.timestamp(), assume that naive datetime instances represent local time and use the appropriate mechanisms in datetime.
  • Don't support this in from_datetime, but provide a slightly clearer error.
  • We could then also add a from_naive_datetime which still allows it, but is more explicit to not make silent mis-assumptions

I tend to the latter variant, but it also bloats up the API somewhat.

I'm currently using

Timestamp.from_posix_seconds(local_begin.timestamp())

which is less than ideal.

Note: the current documentation is already clear, that aware objects are expected.

@tilsche tilsche added enhancement New feature or request discussion There are several possible ways to resolve this issue that need a discussion labels Dec 14, 2022
@tilsche
Copy link
Contributor Author

tilsche commented Mar 2, 2023

I lean towards the third option, but naming it from_local_datetime. And of course also adding better error handling.

tilsche added a commit that referenced this issue Mar 2, 2023
tilsche added a commit that referenced this issue Mar 2, 2023
tilsche added a commit that referenced this issue Mar 2, 2023
tilsche added a commit that referenced this issue Mar 2, 2023
@tilsche tilsche closed this as completed in 372bae7 Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion There are several possible ways to resolve this issue that need a discussion enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant