CLIMATE-467: Replace the existing time decoding function with netCDF4 mod...

[huikyole]

...ules.

[OCWJenkins]

Can one of the admins verify this patch?

[MJJoyce]

test this please

[MJJoyce]

add to whitelist

[OCWJenkins]

Merged build triggered. Test Failed.

[OCWJenkins]

Merged build started. Test Failed.

[OCWJenkins]

Merged build finished. Test Passed.

[MJJoyce]

Hi @huikyole. Sorry for the horrible horrible delay on getting this reviewed. We definitely dropped the ball. I thought I had commented on this already as was awaiting a response back from you. Very sorry! If you have any questions about the comments or would like some help fixing stuff up please feel free to ask.

[huikyole]

That is okay. Thank you for taking care of this finally!

[OCWJenkins]

Merged build triggered. Test Failed.

[OCWJenkins]

Merged build started. Test Failed.

[OCWJenkins]

Merged build finished. Test Failed.

[OCWJenkins]

Merged build triggered. Test Failed.

[OCWJenkins]

Merged build started. Test Failed.

[OCWJenkins]

Merged build finished. Test Passed.

[OCWJenkins]

Merged build triggered. Test Failed.

[OCWJenkins]

Merged build started. Test Failed.

[huikyole]

All of MJJoyce's comments have been addressed. Testing using examples have been successful.

[OCWJenkins]

Merged build finished. Test Passed.

[OCWJenkins]

Merged build triggered. Test Failed.

[OCWJenkins]

Merged build started. Test Failed.

[OCWJenkins]

Merged build finished. Test Passed.

[kwhitehall]

@huikyole This PR needs cleaning.
For e.g. what your tix says, and what work has been done here don't align. Please be certain to include the other functionality attached to this PR e.g. allowing temporal subsetting in your doc string associated with this pull request.
Also, please note that there have been changes to the codebase since this PR was submitted. Given the functionality in the utils class and the replication here, I wonder if one can include the manipulations on the DS times parameter the suggested "temporal_subset" function to the calc_climatology_season in the utils class instead.
Also, it is unclear how the new "spatial_aggregation" functionality is different to that from the Bounds object.

[MJJoyce]

The dataset_processor changes should be in separate tickets since they don't actually relate to this ticket. Would be worthwhile to update this ticket to note that these changes are being made to the handling of all data not labelled monthly as well. The ticket specifies only daily but that isn't accurate to these changes.

As for for the changes to the time decoding it looks fine to me. 👍

[huikyole]

@kwhitehall What do PR and DS stand for? Can we use the word ticket rather than tix?
We need to stop using "calc_climatology_season" which assumes that all OCW datasets are monthly and the length of time dimension is a multiple of 12. The goal of "temporal_subset" is not just to calculate climatology. Seasonal climatology after "temporal_subset" can be calculated using the same module calculating annual climatology.

[OCWJenkins]

Build triggered. Test Failed.

[OCWJenkins]

Build started. Test Failed.

[OCWJenkins]

Build finished. Test Passed.

[MJJoyce]

PR == Pull Request
DS == Dataset

That would be my guess at least

[OCWJenkins]

Merged build triggered. Test Failed.

[OCWJenkins]

Merged build started. Test Failed.

[OCWJenkins]

Merged build finished. Test Failed.

[OCWJenkins]

Merged build triggered. Test Failed.

[OCWJenkins]

Merged build started. Test Failed.

[OCWJenkins]

Merged build finished. Test Passed.