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

Store climo bounds in TimeSet.start_date, end_date for mym files #12

Merged
merged 8 commits into from
Jul 20, 2017

Conversation

rod-glover
Copy link
Contributor

For data files containing multi-year means, we need the climatology bounds as well as the time values. See pacificclimate/climate-explorer-backend#50

Currently, TimeSet.start_date, end_date always contain the naive min and max values of the time variable. For multi-year mean files, this should be the climatological bounds.

This PR:

  • Stores climo bounds for mym files.
  • Splits the currently concatenated climo test file (tiny_climo_gcm) into standards-compliant monthly, seasonal, and yearly test files.
  • Adjusts tests accordingly.

(cherry picked from commit d680b88)
(cherry picked from commit 656c23d)
(Fix merge error.)
Also use datetime.utcnow instead of datetime.now so that modtime
from file system is directly comparable with index time in database

(cherry picked from commit 02555ff)
@rod-glover
Copy link
Contributor Author

@jameshiebert , while you're in a PR-merging mood, do you feel good about this one too?

Copy link
Contributor

@jameshiebert jameshiebert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small comments (one mandatory, one optional), but other than that, it looks great!

@@ -59,6 +59,8 @@
import functools
from argparse import ArgumentParser

from dateutil.relativedelta import relativedelta
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dateutil is an undeclared dependency and should be added to setup.py and requirements.txt.

start_date = climatology_bounds[0][0] + relativedelta(months=1)
end_date = climatology_bounds[-1][1] + relativedelta(months=1)
else:
raise ValueError("Unexpected value '{}' for time_resolution in "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth informing the user as to what you do expect.

@jameshiebert jameshiebert merged commit 7b86845 into master Jul 20, 2017
@jameshiebert jameshiebert deleted the use-climo-bounds-values branch July 20, 2017 16:37
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.

None yet

2 participants