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

Make datetime input more flexible for az_hourly() #62

Merged
merged 47 commits into from
Dec 11, 2023
Merged

Conversation

Aariq
Copy link
Member

@Aariq Aariq commented Oct 20, 2023

Addresses #44 to allow dates alone to be passed to az_hourly(). When a date is passed to end_date_time the results contain data through the end of that date.

This one was tricky because while the AZMet API uses 23:59:59 for midnight rather than 00:00:00, it considers the time interval between 23:00:00 and 23:59:59 exactly one hour. This has made the parse_params() function need to be fairly complicated. Would be happy to simplify things if there are any obvious ways to do so.

The data retrieval functions now print messages whenever parse_params() is "filling in gaps" in inputs. E.g. start and end not supplied, end not supplied, or end not supplied to hour precision (for datetimes).

Other changes:

  • Removes API mocking from most tests—with all the tests using the current date and time added, the mock API requests no longer were useful in many cases. I'll fully remove this infrastructure and instead skip all tests that hit the API on CRAN and if the AZMet API is down in a separate PR.

@Aariq Aariq marked this pull request as ready for review October 20, 2023 19:35
@Aariq Aariq changed the title Make date input more flexible Make datetime input more flexible for az_hourly() Oct 20, 2023
R/az_hourly.R Outdated Show resolved Hide resolved
Copy link
Member

@jeremylweiss jeremylweiss left a comment

Choose a reason for hiding this comment

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

Other than my comment on az_hourly.R, the function works as expected. The changes to datetime input offer a nice added convenience.

@Aariq Aariq linked an issue Oct 24, 2023 that may be closed by this pull request
NEWS.md Outdated Show resolved Hide resolved
Co-authored-by: Jeremy Weiss <[email protected]>
R/parse_params.R Outdated Show resolved Hide resolved
Co-authored-by: Jeremy Weiss <[email protected]>
R/az_hourly.R Outdated Show resolved Hide resolved
R/parse_params.R Outdated Show resolved Hide resolved
Copy link
Member

@jeremylweiss jeremylweiss left a comment

Choose a reason for hiding this comment

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

I'm glad you caught the timezone detail! Also, I like the user feedback messages, which I'm learning are important when it comes to UI-UX-web-data-viz work.

@Aariq
Copy link
Member Author

Aariq commented Nov 30, 2023

Ok! I think this is finally ready for another round of review. Again, you can safely ignore all the changes in .json files in tests/testthat/—these are the mocked API responses.

R/az_daily.R Outdated Show resolved Hide resolved
R/az_daily.R Outdated Show resolved Hide resolved
R/az_daily.R Outdated Show resolved Hide resolved
R/parse_params.R Outdated Show resolved Hide resolved
R/az_hourly.R Outdated Show resolved Hide resolved
R/parse_params.R Outdated Show resolved Hide resolved
Aariq and others added 5 commits December 8, 2023 11:26
Co-authored-by: Jeremy Weiss <[email protected]>
Co-authored-by: Jeremy Weiss <[email protected]>
Co-authored-by: Jeremy Weiss <[email protected]>
Co-authored-by: Jeremy Weiss <[email protected]>
Co-authored-by: Jeremy Weiss <[email protected]>
@Aariq Aariq merged commit 5a08de0 into main Dec 11, 2023
7 checks passed
@Aariq Aariq deleted the date-input-flex branch December 11, 2023 23:30
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.

Allow more flexibility in start and end datetimes for az_hourly()
2 participants