CDAT / cdtime

climate calendar manipulation tools
0 stars 2 forks source link

cdtime add broken #34

Closed lee1043 closed 4 years ago

lee1043 commented 5 years ago

Example in new and old cdms manual describes how add work, as below:

>>> from cdtime import *
>>> c = comptime(1996,2,28)
>>> r = reltime(28,"days since 1996-1-1")
>>> print r.add(1,Day)
29.00 days since 1996-1-1
>>> print c.add(36,Hours)
1996-2-29 12:0:0.0

However, it is not working in my env, as below:

>>> from cdtime import *
>>> c = comptime(1996,2,28)
>>> r = reltime(28,"days since 1996-1-1")
>>> print(r.add(1,Day))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'Day' is not defined
>>> print(c.add(36,Hours))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'Hours' is not defined

It does not work for Months and Years neither. My cdtime version is:

-bash-4.1$ conda list cdtime
# packages in environment at /export_backup/lee1043/anaconda2/envs/pmp_nightly_20181128:
#
# Name                    Version                   Build  Channel
cdtime                    3.1.0.2018.11.01.17.45.gb8c370e  py36h9ac9557_0    cdat/label/nightly

@dnadeau4 @doutriaux1 any idea? Thanks in advance.

doutriaux1 commented 5 years ago

@dnadeau4 we should edit the example to NOT encourage from cdtime import * it is very bad practice

@lee1043 can you try to replace Hours with cdtime.Hours

lee1043 commented 5 years ago

@doutriaux1 how does the cdtime.Hours work? I need a capability to find 180 days later from the specific date using given calendar.

durack1 commented 5 years ago

@lee1043 I've had problems with the cdtime docs before, but did manage to get things working well in durolib - take a peek at durack1/durolib/lib/durolib.py for some options

dnadeau4 commented 5 years ago

Need to expose these "words" to the user in the new __init__.py

File

$CONDA_PREFIX/lib/python3.6/site-packages/cdtime/__init__.py

Code


__all__ = ['__doc__', 'abstime', 'c2r', 'compare', 'componenttime', 'comptime',
        'r2c', 'r2r', 'relativetime', 'reltime', 's2c', 's2r', 'Calendar360',
        'ClimCalendar', 'ClimLeapCalendar', 'Day', 'Days', 'DefaultCalendar',
        'GregorianCalendar', 'Hour', 'Hours', 'JulianCalendar', 'Minute', 'Minutes',
        'MixedCalendar', 'Month', 'Months', 'NoLeapCalendar', 'Season', 'Seasons',
        'Second', 'Seconds', 'StandardCalendar', 'Week', 'Weeks', 'Year', 'Years'
]
dnadeau4 commented 5 years ago

@lee1043 this should do what you want.

import cdtime
r= cdtime.reltime(28,"days since 1996-1-1")
r.add(180,Day).tocomp()
lee1043 commented 5 years ago

@dnadeau4 I modified the $CONDA_PREFIX/lib/python3.6/site-packages/cdtime/__init__.py to include given words as you suggested, but still get error that Day is not defined.

>>> import cdtime
>>> r= cdtime.reltime(28,"days since 1996-1-1")
>>> r.add(180,Day).tocomp()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'Day' is not defined
>>> r.add(180,Days).tocomp()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'Days' is not defined

Am I missing something? Thanks.

dnadeau4 commented 5 years ago

@lee1043 you need cdtime.Days or cdtime.Day, forgot to restart python when I tested it. Now that you changed he init.py, the from cdtime import * will work.

durack1 commented 5 years ago

@dnadeau4 just as a comment, I have found cdtime difficult to use as most of the code is buried in c and not exposed to a user through helpful docs.. I think expanding the documentation to include these example cases that have tripped over users like @lee1043 and myself would be really useful - and even better would be exposing the functions through more helpful docs..

doutriaux1 commented 5 years ago

@durack1 actually I think @dnadeau4 latest version did fix this, most of the C api is now wrapped in Python properly.

durack1 commented 5 years ago

@doutriaux1, OK this is great news, I should download the latest (what is this?) and take another try

lee1043 commented 5 years ago

@dnadeau4 thanks, now I can add days.

>>> import cdtime
>>> r= cdtime.reltime(28,"days since 1996-1-1")
>>> r.add(180,cdtime.Day).tocomp()
1996-7-27 0:0:0.0

But maybe I am missing how to work with calendar:

>>> r.add(180,cdtime.Day,calendar='gregorian').tocomp()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: add() takes no keyword arguments
>>> r.add(180,cdtime.Day,calendar='Calendar360').tocomp()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: add() takes no keyword arguments
>>> r.add(180,cdtime.Day,calendar=cdtime.Calendar360).tocomp()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: add() takes no keyword arguments

Any suggestion? Thanks!

dnadeau4 commented 5 years ago

@lee1043 you cannot do it there. try help(cdtime)

dnadeau4 commented 5 years ago

relative to component function r2c can do it. If I remember the code, I am not sure it is working properly.

cdtime.r2c(r,calendar=cdtime.JulianCalendar)
lee1043 commented 5 years ago

@dnadeau4 but document says I can do it there. First example of table 3.6.1.

t.add(value,intervalUnits, calendar=cdtime.Default-Calendar)
dnadeau4 commented 5 years ago

I just saw that... It probably was able to do it in a previous release. Now the code has not argument. https://github.com/CDAT/cdtime/blob/master/Src/cdtimemodule.c#L496-L499

Documentation

https://cdms.readthedocs.io/en/latest/manual/cdms_3.html#id3

dnadeau4 commented 5 years ago

It is calling this which needs a calendar. If you change the DefaultCalendar that might work.

https://github.com/CDAT/cdtime/blob/master/Src/cdtimemodule.c#L123-L125

dnadeau4 commented 5 years ago

@lee1043 this seems to work...

>>> r= cdtime.reltime(365,"days since 1996-1-1")
>>> r.tocomp()
1996-12-31 0:0:0.0
>>> cdtime.r2c(r,calendar=cdtime.Calendar360)
1997-1-6 0:0:0.0
dnadeau4 commented 5 years ago

The new ways to compare will not work using calendar in PY3.... that is what I remembered earlier. If you can find a solution, just create a PR for me.

reltime_cmp2

https://github.com/CDAT/cdtime/blob/master/Src/cdtimemodule.c#L223

comptime_cmp2

https://github.com/CDAT/cdtime/blob/master/Src/cdtimemodule.c#L423

dnadeau4 commented 5 years ago

@lee1043 This works too...

>>> import cdtime
>>> r= cdtime.reltime(365,"days since 1995-1-1")
>>> r.tocomp(cdtime.Calendar360)
1996-1-6 0:0:0.0
dnadeau4 commented 5 years ago

So the documentation is adding a keyword calendar= which is not needed, the program reads the calendar as an argument.

dnadeau4 commented 5 years ago

Fixed documentation. Should be in master in the next release. https://cdms.readthedocs.io/en/docstanya/manual/cdms_3.html

github-actions[bot] commented 4 years ago

Marking issue as stale, since there has been no activity in 30 days.

Unless the issue is updated or the 'stale' tag is removed, this issue will be closed in 7 days.

durack1 commented 4 years ago

@lee1043 was there anything left to do here?

lee1043 commented 4 years ago

@durack1 nope. I think it's okay to close this.