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

Error message recommends unsupported option #342

Open
TomNicholas opened this issue Jun 19, 2024 · 4 comments
Open

Error message recommends unsupported option #342

TomNicholas opened this issue Jun 19, 2024 · 4 comments

Comments

@TomNicholas
Copy link

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
Copy link
Collaborator

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
Copy link
Collaborator

spencerkclark commented Jun 19, 2024

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
Copy link
Author

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
Copy link
Collaborator

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.

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

2 participants