Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for astronomical year numbering in cftime.datetime #234

Merged
merged 44 commits into from
Mar 25, 2021
Merged

Conversation

jswhit
Copy link
Collaborator

@jswhit jswhit commented Mar 9, 2021

using has_year_zero kwarg. Defaults are set to be consistent with proposed updates to CF (cf-convention/cf-conventions#298). Ignored for idealized calendars like 360_day - these are assumed to always have a year zero.

@jswhit
Copy link
Collaborator Author

jswhit commented Mar 11, 2021

@spencerkclark - I would appreciate it if you could look this over if you have time.

@jswhit
Copy link
Collaborator Author

jswhit commented Mar 11, 2021

#233

@spencerkclark
Copy link
Collaborator

Sure thing — I’ll try and have a look tomorrow.

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.

It is cool to add the flexibility to do this to cftime! I played around with things a bit and I have two suggestions, and also perhaps a high-level design question.

  1. To be safe, I think we should not allow subtracting dates with different year zero conventions from each other; e.g. this is confusing behavior:
>>> d1 = cftime.datetime(-4712, 1, 2, calendar="julian", has_year_zero=False)
>>> d2 = cftime.datetime(-4710, 1, 2, calendar="julian", has_year_zero=True)
>>> d2 - d1
datetime.timedelta(days=365)
>>> d1 = cftime.datetime(-4712, 1, 2, calendar="julian", has_year_zero=False)
>>> d2 = cftime.datetime(-4710, 1, 2, calendar="julian", has_year_zero=False)
>>> d2 - d1
datetime.timedelta(days=730)
  1. Similarly, I think dates with different year zero conventions should not be equal (or comparable in other ways):
>>> d1 = cftime.datetime(-4712, 1, 2, calendar="julian", has_year_zero=False)
>>> d2 = cftime.datetime(-4712, 1, 2, calendar="julian", has_year_zero=True)
>>> d1 == d2
True

In terms of design, while checks for (1) and (2) could be added in the current structure, I wonder if at a high level (while they might share a lot of the same low-level code) it might make more sense to define these new options as different calendars (e.g. "julian" versus "julian_astronomical" or something like that). In other words, remove the has_year_zero argument and fold its functionality into new options for the existing calendar argument in the cftime.datetime constructor and other high-level cftime functions like num2date and date2num.

I think this would allow xarray, and perhaps other packages, to more easily support these kinds of dates (because it would fit into our current approach for encoding and decoding times to and from files, where we rely only on the CF-convention-required calendar and units attributes).

What would your thoughts be on that?

@jswhit
Copy link
Collaborator Author

jswhit commented Mar 12, 2021

Good point about comparisons with different year zero conventions. We could create new calendars for year zero, but that would diverge from the CF metadata conventions. This recent proposal would allow for year zero in the proleptic_gregorian, but not in gregorian or julian. If this is adopted, my thought was that we could handle this by changing the calendar specific defaults for has_year_zero.

@jswhit
Copy link
Collaborator Author

jswhit commented Mar 12, 2021

I guess what I'm saying is since CF is not creating new calendar types then it will still be possible for xarray to do what it needs to do with just the units and calendar attributes. The calendar-specific default values of has_year_zero will handle all the CF possibilities. The has_year_zero kwarg will only need to be used if you want to deviate from the CF conventions.

@spencerkclark
Copy link
Collaborator

That makes sense -- is the reason for supporting things that deviate from CF conventions for backwards compatibility if/when the conventions change?

@jswhit
Copy link
Collaborator Author

jswhit commented Mar 12, 2021

That makes sense -- is the reason for supporting things that deviate from CF conventions for backwards compatibility if/when the conventions change?

Yes, plus allowing year zero when CF doesn't allow it has been requested before (mainly by folks in the astronomy community). We can also adapt easily to future changes CF conventions by just modifying the defaults.

EDIT: I went ahead and changed the default for proleptic_gregorian to True, consistent with the proposed update to CF (cf-convention/cf-conventions#298) since this is consistent with ISO 8601. Strictly speaking, dates before 1-1-1 will be disallowed in the CF update for julian and gregorian/standard calendars, but cftime does not enforce this. Should we?

@spencerkclark
Copy link
Collaborator

Yes, plus allowing year zero when CF doesn't allow it has been requested before (mainly by folks in the astronomy community).

Right, and so for now we can take the position that if the astronomy folks want to encode these dates in netCDF files in any universally recognized way (e.g. in xarray), then they will need to make their case to the CF community. I think that makes sense (I think I was getting a bit ahead of myself above).

I went ahead and changed the default for proleptic_gregorian to True, consistent with the proposed update to CF (cf-convention/cf-conventions#298) since this is consistent with ISO 8601. Strictly speaking, dates before 1-1-1 will be disallowed in the CF update for julian and gregorian/standard calendars, but cftime does not enforce this. Should we?

Indeed the backwards compatibility issue is a bit tricky. I do not know how common the use of negative years is in Julian and Gregorian/standard calendars throughout the climate modeling community. In the case of the Julian calendar we have at least this example (#71). In an ideal world I think we should enforce this -- at least from the xarray perspective it would be useful to guard against users creating CF-invalid files, but should we go through some sort of deprecation cycle first?

@jswhit
Copy link
Collaborator Author

jswhit commented Mar 15, 2021

There are lots of ways that users can create netcdf files with non-CF compliant metdadata - I don't think it's up to us to enforce this. Perhaps we could just issue a warning if a CF-invalid date is created?

EDIT: I added a warning if a CF invalid date is requested.

@jswhit
Copy link
Collaborator Author

jswhit commented Mar 25, 2021

since the CF convention change is delayed I've restored the previous default behavior so we can get this merged.

@kaos000
Copy link

kaos000 commented Dec 20, 2023

Hi, I am fumbling my way through working with NetCDF files in R and have a large palaeoclimate.nc that has this time dimension:
time Size:11500 *** is unlimited ***
standard_name: time
units: hours since 1-1-1 00:00:00
calendar: standard
axis: T

The thing is, I would like to reformat the time units so they properly reflect BP. I assume I have to change the calendar as well.
Right now, the range of time is only 1000 years, so the hours since 1-1-1 00:00:00 reset for each 1000-year span, making it difficult to create a time series.
Is there a straightforward way to accomplish this? Or do I need the author of the file to recode it?
Thanks!
K

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.

3 participants