Skip to content

have utime.num2date/utime.date2num call module level functions#180

Merged
jswhit merged 37 commits intomasterfrom
no_utime
Jul 1, 2020
Merged

have utime.num2date/utime.date2num call module level functions#180
jswhit merged 37 commits intomasterfrom
no_utime

Conversation

@jswhit
Copy link
Collaborator

@jswhit jswhit commented Jun 24, 2020

JulianDayFromDate, DateFromJulianDay no longer used internally.
Bump version number to 1.2.0.

@jswhit
Copy link
Collaborator Author

jswhit commented Jun 29, 2020

@spencerkclark - your latest improvements to date2num and num2date work so well that I decided to go ahead and use them everywhere. Would you mind running this with the xarray test suite? If it all looks good, I will merge and create a 1.2.0 release.

Copy link
Collaborator

@spencerkclark spencerkclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow thanks @jswhit -- this is great. The xarray test suite passes with the code as written, but I have a few questions/suggestions.

One thing I also noticed while playing around with this -- you could address it here or I could open a separate PR -- is that the repr updated in #177 does not include the minutes of the datetime. I'm guessing this was a simple mistake, but would probably be good to fix before a release!

cftime/cftime/_cftime.pyx

Lines 1677 to 1680 in ca851ca

def __repr__(self):
return "{0}.{1}({2}, {3}, {4}, {5}, {6}, {7})".format('cftime',
self.__class__.__name__,
self.year,self.month,self.day,self.hour,self.second,self.microsecond)

This results in confusing reprs such as:

In [1]: import cftime

In [2]: cftime.num2date([0, 15, 30], units="minutes since 2000-01-01", calendar="noleap")
Out[2]:
array([cftime.DatetimeNoLeap(2000, 1, 1, 0, 0, 0),
       cftime.DatetimeNoLeap(2000, 1, 1, 0, 0, 0),
       cftime.DatetimeNoLeap(2000, 1, 1, 0, 0, 0)], dtype=object)

Copy link
Collaborator

@spencerkclark spencerkclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that repr bug! Fixed now, along with the masked array bug.
Getting rid of the loop in date2num is going to be trickier, since there are so may checks/if tests in there. Going to leave that for another PR.

Sounds good! Thanks for the fixes. This looks good to me; I confirmed again that the xarray test suite passes with these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments