Implement fromutc for FixedOffset

[movermeyer]

What are you trying to accomplish?

Fixes #108. cc @davidkraljic @deargle

What approach did you choose and why?

The docs are a little inconsistent when they describe what the behaviour of dst should be for fixed offset tzinfo objects:

Return the daylight saving time (DST) adjustment, as a timedelta object or None if DST information isn’t known.

The way I think about fixed offset tzinfo objects, is that they don't know any DST information, so they should return None.
Indeed, that is what the timezone_dst upstream code does.

However, the docs continue to say...

Most implementations of dst() will probably look like one of these two:

def dst(self, dt):
    # a fixed-offset class:  doesn't account for DST
    return timedelta(0)

Which suggests that a fixed-offset implementation should return timedelta(0).

Indeed, the default implementation of fromutc checks to ensure that dst does not return None (which is #108).
The timezone implementation avoids this by implementing its own fromutc.

---

So that covers the two options I had to fix this...

1. Change the return value of FixedOffset.dst to return timedelta(0)
2. Keep the return value as None, but implement a fromutc method

I chose the latter, since I want to maintain exact behaviour with timezone. It also matches my understanding of how fixed offset implementations should behave; returning timedelta(0) implies that it knows something about DST, which it does not.

I copied the upstream implementation , and made the minimal modifications so that it would work outside of the cPython codebase.

---

In the process, I uncovered a few more latent bugs:

• You were able to call utcoffset, dst, and tzname without any parameters, which is wrong but went unnoticed since none of those methods actually use the parameters (and the methods were marked as METH_VARARGS and not METH_O.
• tzname in Python 2.7 needs to return a str, when it was returning a unicode

What should reviewers focus on?

🤷

[davidkraljic]

Thank you!

[thomasst]

This looks good.

But I don't understand why fromutc() works this way in general.

tzinfo.fromutc(dt)
This is called from the default datetime.astimezone() implementation. When called from that, dt.tzinfo is self, and dt’s date and time data are to be viewed as expressing a UTC time.

If it views the dt as expressing UTC time, why does it expect a datetime with a non-UTC tzinfo? Shouldn't it just take an unaware datetime? Just doesn't make sense to me or I'm missing something. But datetimes and timezones in Python are hard.

[movermeyer]

But datetimes and timezones in Python are hard.

Yeah, the design doesn't seem to be all that sensible. That's why it took me so many weeks to make this fairly simple fix. Every time I read the docs I got confused by the disconnect between what the docs say and the actual implementation code in cpython.

At this point, I'm fairly confident in what I've done here, and stuck to what the implementation of timezone does, since that's what we're trying to mimic with FixedOffset.

[thomasst]

Looks like it isn't building: https://app.circleci.com/pipelines/github/closeio/ciso8601/175/workflows/346aa53f-c46a-41c0-997d-03ac8f8adbb0/jobs/2309

[movermeyer]

Looks like it isn't building: https://app.circleci.com/pipelines/github/closeio/ciso8601/175/workflows/346aa53f-c46a-41c0-997d-03ac8f8adbb0/jobs/2309

@thomasst It's been failing since 2021-11-14. It's unrelated to this PR. I'll look into it.

Update: Here is the fix.