4 ms·

I don't understand why the functions weren't just made to return non-naive results. You've literally asked for UTC. There's no excuse for allowing the result of

I don't understand why the functions weren't just made to return non-naive results. You've literally asked for UTC. There's no excuse for allowing the result of that to be interpreted as something that isn't UTC. Set a separate flag that governs conversion to string to avoid different ISO output for shitty string parsers if you want, but there's no great reason to not just fix the functions so that they continue to work better than before and do the thing that people actually expect them to do, which is produce UTC. Intentionally naive storage or transfer is irrelevant. That's just serialization. And when you explicitly tell the computer to "deserialize this as UTC" it should fucking do what you told it to and give a zone aware result.

ak217 3y ago
Let's imagine that I have a perfectly reasonable looking piece of code in my app: db.write(Event(timestamp=int(datetime.datetime.utcnow().timestamp())) After the change that you propose, the timestamps written by this code will jump by an amount equal to the offset of the local timezone this machine runs on (and also lose any DST adjustment they might have had). Now, let's imagine another independent process is consuming events from the database and compares an event written before this change to one written after. Python is a popular language, used in production by scheduling applications around the world. The potential downside of this kind of change is measured in billions of dollars. The safe solution is of course to not silently change the API in a breaking way, and instead introduce a new API and get someone to look at why the code stopped working.
- wodenokoto 3y ago
  On my machine, a naive timestamp from utcnow() and a utc timezone aware datetime object from datetime.now(tz=ZoneInfo('utc')) produces the same timestamp. datetime.now() does not return utc time (but does return a naive datetime object)
  - ak217 3y ago
    OK, but that's not relevant to the scenario that I described.
    - wodenokoto 3y ago
      How is that not exactly the scenario?
raverbashing 3y ago
For real, as much as I'm annoyed with the change, this is how it should have been It is kinda like the 2->3 change with strings And utcnow is a bad name. But the "fix" is none the better
drkevorkian 3y ago
This would be a backwards incompatible change that would cause a lot of issues. For instance, you are not allowed to compare naive and non-naive datetimes, so for instance, `utcnow() > datetime(2023, 11, 19)` would work before, but break following your change.
- ak217 3y ago
  Breaking following the change is fine. Silently changing the behavior in a way that could cause catastrophic bugs is not. That's why the suggestion to just keep the functions and change their behavior is naive. It would obviously be very unsafe to do so.
- akersten 3y ago
  Removing the functions wholesale isn't a backwards incompatible change?
  - ollien 3y ago
    It is one that can be found via static analysis. That may not be true if behavior is changed. In some cases, this may be "ok" (see: Go's recent loop changes), but in others, it may not.
    - IshKebab 3y ago
      Most Python users don't do any form of static analysis though, so both changes would only be found at runtime. But I think changing the behaviour and not the name is still a very bad idea.
      - kevincox 3y ago
        Yes, but "Unknown function datetime.utcnow" is much easier to diagnose than somewhere in your application, possibly inside a third-party library or maybe even in a different service altogether (because the change traversed serialization and deserilialization) throwing an exception because you can't compare two datetimes.
InitialBP 3y ago
If this was something used only by your company or a specific project, then I’m absolutely on board. However, this is in a library used in thousands (millions?) of projects. Since it’s potentially a breaking change you can’t just change it and expect everyone to know. Many people won’t know it’s deprecated until the code fails to run, and having new functions rather than changing the functionality of existing functions makes it much easier to identify what is actually happening.
- ak217 3y ago
  Code failing to run is the good outcome.
thrdbndndn 3y ago
> I don't understand why the functions weren't just made to return non-naive results. You've literally asked for UTC. I totally agree with you that it makes no sense this `utcnow()` does not return tz-aware object. But I think their fix is great in this regard: now you just use `datetime.now(timezone.utc)`. It's more explicit, it returns aware DT object now, and more importantly, it gets rid of a function that I always feel too specific to have anyway (like how we don't have `plus(a, 2)` and `plustwo(a)`).
- abh123 3y ago
  I got bit by this once. Timezones aware date calculations can be an order of magnitude slower than non tz aware. Consider every time a legislative body changes dst, it creates an entry in the timezone database that needs to be checked when adding or subtracting time from that timezone.
  - ryukoposting 3y ago
    This is one of the reasons why Unix epoch time soldiers on, even though it is totally indecipherable to humans. It can be easily mapped to a timezone-aware type, but performing arithmetic on it is trivial.
- sanderjd 3y ago
  Except isn't datetime.now() still going to return a tz-naive datetime? I would much rather have a method that requires an explicit tz, or explicitly uses UTC (like utcnow, but with the right return type). Looking for this mistake in code reviews gets really old.

ak217 3y ago

Let's imagine that I have a perfectly reasonable looking piece of code in my app: db.write(Event(timestamp=int(datetime.datetime.utcnow().timestamp())) After the change that you propose, the timestamps written by this code will jump by an amount equal to the offset of the local timezone this machine runs on (and also lose any DST adjustment they might have had). Now, let's imagine another independent process is consuming events from the database and compares an event written before this change to one written after. Python is a popular language, used in production by scheduling applications around the world. The potential downside of this kind of change is measured in billions of dollars. The safe solution is of course to not silently change the API in a breaking way, and instead introduce a new API and get someone to look at why the code stopped working.

wodenokoto 3y ago
On my machine, a naive timestamp from utcnow() and a utc timezone aware datetime object from datetime.now(tz=ZoneInfo('utc')) produces the same timestamp. datetime.now() does not return utc time (but does return a naive datetime object)
- ak217 3y ago
  OK, but that's not relevant to the scenario that I described.
  - wodenokoto 3y ago
    How is that not exactly the scenario?

raverbashing 3y ago

For real, as much as I'm annoyed with the change, this is how it should have been It is kinda like the 2->3 change with strings And utcnow is a bad name. But the "fix" is none the better

drkevorkian 3y ago

This would be a backwards incompatible change that would cause a lot of issues. For instance, you are not allowed to compare naive and non-naive datetimes, so for instance, `utcnow() > datetime(2023, 11, 19)` would work before, but break following your change.

ak217 3y ago
Breaking following the change is fine. Silently changing the behavior in a way that could cause catastrophic bugs is not. That's why the suggestion to just keep the functions and change their behavior is naive. It would obviously be very unsafe to do so.
akersten 3y ago
Removing the functions wholesale isn't a backwards incompatible change?
- ollien 3y ago
  It is one that can be found via static analysis. That may not be true if behavior is changed. In some cases, this may be "ok" (see: Go's recent loop changes), but in others, it may not.
  - IshKebab 3y ago
    Most Python users don't do any form of static analysis though, so both changes would only be found at runtime. But I think changing the behaviour and not the name is still a very bad idea.
    - kevincox 3y ago
      Yes, but "Unknown function datetime.utcnow" is much easier to diagnose than somewhere in your application, possibly inside a third-party library or maybe even in a different service altogether (because the change traversed serialization and deserilialization) throwing an exception because you can't compare two datetimes.

InitialBP 3y ago

If this was something used only by your company or a specific project, then I’m absolutely on board. However, this is in a library used in thousands (millions?) of projects. Since it’s potentially a breaking change you can’t just change it and expect everyone to know. Many people won’t know it’s deprecated until the code fails to run, and having new functions rather than changing the functionality of existing functions makes it much easier to identify what is actually happening.

ak217 3y ago
Code failing to run is the good outcome.

thrdbndndn 3y ago

> I don't understand why the functions weren't just made to return non-naive results. You've literally asked for UTC. I totally agree with you that it makes no sense this `utcnow()` does not return tz-aware object. But I think their fix is great in this regard: now you just use `datetime.now(timezone.utc)`. It's more explicit, it returns aware DT object now, and more importantly, it gets rid of a function that I always feel too specific to have anyway (like how we don't have `plus(a, 2)` and `plustwo(a)`).

abh123 3y ago
I got bit by this once. Timezones aware date calculations can be an order of magnitude slower than non tz aware. Consider every time a legislative body changes dst, it creates an entry in the timezone database that needs to be checked when adding or subtracting time from that timezone.
- ryukoposting 3y ago
  This is one of the reasons why Unix epoch time soldiers on, even though it is totally indecipherable to humans. It can be easily mapped to a timezone-aware type, but performing arithmetic on it is trivial.
sanderjd 3y ago
Except isn't datetime.now() still going to return a tz-naive datetime? I would much rather have a method that requires an explicit tz, or explicitly uses UTC (like utcnow, but with the right return type). Looking for this mistake in code reviews gets really old.