fix: remove milliseconds from isoformat

[iloveitaly]

Fixes #841

[dbanty]

The trouble is, this only works when the user supplies a datetime with no timezone. If the user supplies a timezone, the appended Z would be incorrect.

We also probably don't want to add in Z when there is no timezone, because the user may not have created a UTC time, but a local one.

Unfortunately I don't think there's a way to indicate using the standard library types that a timezone is required, so the only way to really solve this in a way that's obvious to users might be to create a new custom date class... but also it would be really annoying to not allow standard datetime as input. Only other option I can think of is throwing an exception, which I generally prefer not to.

[iloveitaly]

Yeah that all makes sense. I don't have a good grasp of the project enough to understand the best way to handle it, but a custom class feels like the right approach. Maybe coupled with a warning log when a datetime is encountered.

Could you point me to the right place to add a custom class and how to use it when generating?

[dogukancagatay]

If the provided datetime object does not contain timezone data, it can be logically assumed that it is in the computer's local timezone. This would make isoformat() always output an RFC3339-compliant string.

[iloveitaly]

it can be logically assumed that it is in the computer's local timezone

I'm not sure if this assumption works—I think in many cases the assumption is UTC if no TZ is defined.

[dbanty]

I'm not sure if this assumption works—I think in many cases the assumption is UTC if no TZ is defined.

That's certainly the challenge 😓. There's a lot of code in the wild using datetime.utcnow() which creates a datetime with no tzinfo. That's deprecated as of Python 3.12, but it'll be quite a while before everyone is upgraded to see that warning. Eventually once datetime.now() is the main way people make naive datetimes, we could probably assume local.

Could you point me to the right place to add a custom class and how to use it when generating?

The problem with this approach is that it's going to add a ton of boilerplate to clients that are already using datetime successfully with timezones. So seems like we should make this an opt-in feature if we go this route, not the default. And that is gonna make the code quite messy. And in a few years when newer Python versions are in higher use, this approach probably won't be needed anymore. So I've kind of talked myself out of this.

We could also start throwing an exception if a datetime doesn't have a timezone rather than encoding the need for a timezone in the type system. That's probably more Pythonic (even if I personally hate it 😆).

One other note—looks like RFC-3339 does allow for fractional seconds (milliseconds) so we wouldn't have to remove those, just require a timezone.