Unidata / cftime

Time-handling functionality from netcdf4-python.
https://unidata.github.io/cftime
MIT License
75 stars 39 forks source link

Error message recommends unsupported option #342

Open TomNicholas opened 4 months ago

TomNicholas commented 4 months ago

When I try to use the num2date function to compute "years since ..." it recommends that I can use common_years as the unit as long as I set calendar='noleap'. However this doesn't seem to work (with an opaque error), and is also inconsistent with the docstring for num2date which says

common_years since is allowed only for the 365_day calendar

Screenshot 2024-06-19 at 11 15 24 AM

Either the error message or the docstring must be incorrect.


To report a non-security related issue, please provide:

  • the version of the software with which you are encountering an issue

cftime 1.6.3

  • environmental information (i.e. Operating System, compiler info, java version, python version, etc.)

python 3.10

  • a description of the issue with the steps needed to reproduce it

See screenshot above

spencerkclark commented 4 months ago

I think a more informative error could be raised, but this looks like a date parsing issue rather than an issue with the units (cftime does not know how to interpret "0347-01" as a date):

>>> cftime.num2date(0, units="common_years since 0347-01", calendar="noleap")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "src/cftime/_cftime.pyx", line 587, in cftime._cftime.num2date
  File "src/cftime/_cftime.pyx", line 113, in cftime._cftime._dateparse
  File "src/cftime/_cftime.pyx", line 791, in cftime._cftime._parse_date
TypeError: int() argument must be a string, a bytes-like object or a real number, not 'NoneType'

Switching to "0347-01-01" allows things to work:

>>> cftime.num2date(0, units="common_years since 0347-01-01", calendar="noleap")
cftime.DatetimeNoLeap(347, 1, 1, 0, 0, 0, 0, has_year_zero=True)
spencerkclark commented 4 months ago

Unrelated, but I couldn't help but notice in your example that you are passing a np.datetime64 value to cftime.num2date. It probably speaks to having examples in the documentation (#343), but this is not the use-case of num2date :).

num2date expects numeric data (scalar values or arrays of integers or floats) and returns cftime objects. date2num is the reverse, taking cftime objects and converting to numeric values.

TomNicholas commented 4 months ago

Thank you @spencerkclark ! Clearly I had misunderstood what num2date does. I've got my code working now thanks to you.

It probably speaks to having examples in the documentation

Yes the correct usage would have been more obvious with an example.

Switching to "0347-01-01" allows things to work:

cftime.num2date(0, units="common_years since 0347-01-01", calendar="noleap") cftime.DatetimeNoLeap(347, 1, 1, 0, 0, 0, 0, has_year_zero=True)

This behaviour still seems to me to be inconsistent with the docstring though, which says

common_years since is allowed only for the 365_day calendar.

spencerkclark commented 4 months ago

Great, I'm glad you got what you needed to working!

This behaviour still seems to me to be inconsistent with the docstring though, which says

common_years since is allowed only for the 365_day calendar.

365_day and noleap are synonyms, but I agree, this could be made clearer in the docstring.