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

Guess start and end date when no time bounds are available #116

Merged
merged 3 commits into from
Sep 28, 2023

Conversation

dougiesquire
Copy link
Collaborator

With this PR, when no time bounds are available in a file, a guess of the start and end dates are made based on the first and last time and the frequency.

@aidanheerdegen, @utkarshgupta95 this is definitely a yucky hack, but it should fix #113 and #114.

Closes #113, closes #114

@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (f7ca779) 97.31% compared to head (4024c97) 97.31%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
- Coverage   97.31%   97.31%   -0.01%     
==========================================
  Files           9        9              
  Lines         522      559      +37     
==========================================
+ Hits          508      544      +36     
- Misses         14       15       +1     
Files Coverage Δ
src/access_nri_intake/source/utils.py 98.49% <98.24%> (-0.47%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@jo-basevi jo-basevi left a comment

Choose a reason for hiding this comment

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

Hi Dougie, Aidan suggested that I have a look at this PR as I have done some similar date calculations with date-based restart pruning in Payu. From what I can tell, the changes all look good to me!

I'm unfamiliar with the xarray data (so I might not be the best person for this review sorry!), is the time data points normally at the start or end times, or is it impossible to know if there isn't known bounds? Just wondering if its better to have end/start times to be either +/- the frequency rather than half the frequency either side of start and end times.

src/access_nri_intake/source/utils.py Outdated Show resolved Hide resolved
src/access_nri_intake/source/utils.py Show resolved Hide resolved
@dougiesquire
Copy link
Collaborator Author

Thanks heaps @jo-basevi!

is the time data points normally at the start or end times, or is it impossible to know if there isn't known bounds? Just wondering if its better to have end/start times to be either +/- the frequency rather than half the frequency either side of start and end times.

I don't think it's possible to know where the time point is, unfortunately. However, I think the assumption that the time point is at centre of the averaging period makes the most sense. This is indeed the case for the data in the issues that motivated this PR (#113, #114).

@dougiesquire
Copy link
Collaborator Author

RTD build is now failing. This PR does not update the docs, and I've opened an issue here #118, so merging

@dougiesquire dougiesquire merged commit b891b5c into main Sep 28, 2023
13 of 15 checks passed
@dougiesquire dougiesquire deleted the 113_guess_start_end_date branch September 28, 2023 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants