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

always create cftime datetime instances by default in num2date #136

Closed
jswhit opened this issue Nov 22, 2019 · 4 comments
Closed

always create cftime datetime instances by default in num2date #136

jswhit opened this issue Nov 22, 2019 · 4 comments

Comments

@jswhit
Copy link
Collaborator

jswhit commented Nov 22, 2019

Currently, a python datetime instance is returned if possible. This can be controlled by the use_only_cftime_datetimes kwarg, which is currently False by default. PR #135 switches the default to True to avoid surprising results such as discussed in issue #134.

Will this cause problems with existing code? Is anyone relying on the current default?

@jswhit jswhit changed the title always create cftime datetime instances in num2date always create cftime datetime instances by default in num2date Nov 22, 2019
@spencerkclark
Copy link
Collaborator

spencerkclark commented Nov 22, 2019

At least from the perspective of xarray this should not be a problem; we already always use use_only_cftime_datetimes=True.

@davidhassell
Copy link
Contributor

cf-python uses use_only_cftime_datetimes=True, as well, so no problem for us.

@jswhit
Copy link
Collaborator Author

jswhit commented Nov 26, 2019

PR #135 merged. use_only_cftime_datetimes=True is now the default.

@5tefan
Copy link
Contributor

5tefan commented Jul 20, 2020

I have to complain that the way this change was implemented has been a bit annoying. To maintain compatibility between pre and post cftime 1.1 versions as we upgrade various systems I have resorted to do:

            try:
                time_dt = nc.num2date(time, nc_in.variables["time"].units, 
                        only_use_cftime_datetimes=False,
                        only_use_python_datetimes=True)
            except TypeError:
                # backwards compat with cftime less than 1.1, fall back with no kwargs
                # TypeError: num2date() got an unexpected keyword argument only_use_python_datetimes
                time_dt = nc.num2date(time, nc_in.variables["time"].units)

Since cftime was strict with kwargs in earlier versions, the new kwargs only_use_cftime_datetimes and only_use_python_datetimes are not accepted.

At least several libraries we use break with DatetimeGregorian, including so far spacepy and matplotlib, and we're only starting to upgrade. Am I misunderstanding something here?

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

No branches or pull requests

4 participants