nightshade UTC check

[alpha-beta-soup]

When trying to use the basemap nightshade method I noticed a bug in how basemap checks if the supplied datetime is in UTC. The relevant place in the codebase is here

# make sure date is utc.
if date.utcoffset() is not None:
    raise ValueError('datetime instance must be UTC')

However datetime objects that are in UTC and are aware of it do not pass this check:

In [1]: import datetime

In [2]: import pytz

In [3]: d = datetime.datetime.now()

In [4]: d
Out[4]: datetime.datetime(2016, 2, 29, 18, 21, 56, 388082)

In [5]: d.utcoffset() is not None
Out[5]: False

In [6]: d = d.replace(tzinfo=pytz.UTC)

In [7]: d
Out[7]: datetime.datetime(2016, 2, 29, 18, 21, 56, 388082, tzinfo=<UTC>)

In [8]: d.utcoffset() is not None
Out[8]: True

So even though my datetime has been made timezone aware (and converted from the original timezone to UTC), I have to set the timezone to None in order to pass the check for whether it is UTC... in which case it is strictly no longer UTC (just assumed to be so).

[WeatherGod]

hmm, seems like that check could simply be replaced with "if
date.utcoffset():"?

On Mon, Feb 29, 2016 at 12:23 AM, Richard Law notifications@github.com
wrote:

When trying to use the basemap nightshade method I noticed a bug in how
basemap checks if the supplied datetime is in UTC. The relevant place in
the codebase is here

basemap/lib/mpl_toolkits/basemap/__init__.py

Line 4703 in 8872dbc

# make sure date is utc.

make sure date is utc.if date.utcoffset() is not None:

raise ValueError('datetime instance must be UTC')

However datetime objects that are in UTC and are aware of it do not pass
this check:

In [1]: import datetime

In [2]: import pytz

In [3]: d = datetime.datetime.now()

In [4]: d
Out[4]: datetime.datetime(2016, 2, 29, 18, 21, 56, 388082)

In [5]: d.utcoffset() is not None
Out[5]: False

In [6]: d = d.replace(tzinfo=pytz.UTC)

In [7]: d
Out[7]: datetime.datetime(2016, 2, 29, 18, 21, 56, 388082, tzinfo=)

In [8]: d.utcoffset() is not None
Out[8]: True

So even though my datetime has been made timezone aware (and converted
from the original timezone to UTC), I have to set the timezone to None in
order to pass the check for whether it is UTC... in which case it is
strictly no longer UTC (just assumed to be so).

—
Reply to this email directly or view it on GitHub
#272.

[micahcochran]

I think the intent of the test is so that someone doesn't accidentally set it to a particular time zone (make it an "aware" datetime), which is different from what the comment # make sure date is utc. suggests.

A couple extra examples to add to the mix:

>>> datetime.datetime.now().utcoffset() is None
True

# Implemented as "naive" for backward compatibility?
>>> datetime.datetime.utcnow().utcoffset() is None
True

In [48]: datetime.datetime.utcnow().replace(tzinfo=pytz.UTC).utcoffset()
Out[48]: datetime.timedelta(0)

Could that test be changed to this?

# checks if date is an aware datetime and if that timezone is set to UTC
if date.utcoffset() is not None and date.utcoffset() != dateime.timedelta(0):
    raise ValueError('datetime instance must be UTC')

If my logic is correct it should test that you haven't set this to some other timezone "aware". It would still accept the naive time zone when tzinfo is not set. It would accept if you have explicitly set it to UTC time.

[WeatherGod]

Looks like timedelta() respects the non-zero semantics:

from datetime import timedelta
bool(timedelta(0))
False
bool(timedelta(1))
True

So, we can simplify that check a bit. Does anyone want to put together the
PR for that?

On Mon, Feb 29, 2016 at 11:57 AM, Micah Cochran notifications@github.com
wrote:

I think the intent of the test is so that someone doesn't accidentally set
it to a particular time zone (make it an "aware" datetime), which is
different from what the comment # make sure date is utc. suggests.

A couple extra examples to add to the mix:

datetime.datetime.now().utcoffset() is NoneTrue

Implemented as "naive" for backward compatibility?>>> datetime.datetime.utcnow().utcoffset() is NoneTrue

In [48]: datetime.datetime.utcnow().replace(tzinfo=pytz.UTC).utcoffset()
Out[48]: datetime.timedelta(0)

Could that test be changed to this?

checks if date is an aware datetime and if that timezone is set to UTCif date.utcoffset() is not None and date.utcoffset() != dateime.timedelta(0):

raise ValueError('datetime instance must be UTC')

If my logic is correct it should test that you haven't set this to some
other timezone "aware". It would still accept the naive time zone when
tzinfo is not set. It would accept if you have explicitly set it to UTC
time.

—
Reply to this email directly or view it on GitHub
#272 (comment).

[alpha-beta-soup]

I think even clearer is

if date.utcoffset() is not None and date.utcoffset().total_seconds() != 0:
    raise ValueError('datetime instance must be UTC, not {}'.format(date.tzname()))

I'll put together a PR now. I'm not sure if you want that addendum to the exception, so let me know.

[alpha-beta-soup]

Sorry, just saw the last comment. I'll implement that.

[alpha-beta-soup]

I don't know if there are any tests for nightshade? It would seem as if there at least also needs to be the case of using a datetime with a declared tzinfo. I'll check/modify these tests if someone can point me to any existing tests for nightshade.

[WeatherGod]

Yeah, there are no tests for most of basemap. Previously, the examples were
simply spot-checked, I believe. There is a testing framework, but I don't
think we import dateutils.

On Mon, Feb 29, 2016 at 3:12 PM, Richard Law notifications@github.com
wrote:

I don't know if there are any tests for nightshade? It would seem as if
there at least also needs to be the case of using a datetime with a
declared tzinfo. I'll check/modify these tests if someone can point me to
any existing tests for nightshade.

—
Reply to this email directly or view it on GitHub
#272 (comment).