-
Notifications
You must be signed in to change notification settings - Fork 0
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
Changed 'wind_2min_timestamp' variable type from character to date-ti… #52
Changes from 2 commits
c58cd0d
50e9c94
db866c4
cd866fa
8c49f1a
db131a1
df66f73
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,7 +91,8 @@ az_hourly <- function(station_id = NULL, start_date_time = NULL, end_date_time = | |
as.numeric | ||
)) %>% | ||
dplyr::filter(.data$meta_station_id != "az99") %>% | ||
dplyr::mutate(date_datetime = lubridate::ymd_hms(.data$date_datetime)) %>% | ||
dplyr::mutate(date_datetime = lubridate::force_tz(lubridate::ymd_hms(.data$date_datetime), tzone = "America/Phoenix")) %>% | ||
dplyr::mutate(wind_2min_timestamp = lubridate::with_tz(lubridate::parse_date_time(.data$wind_2min_timestamp, orders = "ymdHMSz"), tzone = "America/Phoenix")) %>% | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the difference between There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My understanding is that both handle With With There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NEWS.md file updated There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you commit and push the NEWS.md changes? I don't see them in this pull request There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, they're there now. But, I managed to make a conflict There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can show you how to deal with the merge conflict next time we meet (next Friday), or you can pick a time tomorrow or earlier next week: https://calendar.app.google/eCWobQteVLViq5NJ8 |
||
#convert NAs | ||
dplyr::mutate( | ||
dplyr::across( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just below this is code to change values like -9999 to NA, but it currently skips
wind_2min_timestamp
. I think all you need to do is:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without adding the above code to include
wind_2min_timestamp
in the conversion of NAs, existing code withwind_2min_timestamp
as character automatically converts correspondingnodata
values of-99999
toNA
while throwing a warning27 failed to parse
, as withaz_daily(start_date = "2023-03-22", end_date = "2023-03-22")
Only one of the 28 stations at that point had
wind_2min_timestamp
daily values on that date.I think the
nodata
value conversion needs to happen somehow in conjunction with character-to-datetime conversion of this variable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the above code has trouble with
wind_2min_timestamp
, as it is an object of class "c('POSIXct', 'POSIXt')". This is after the character-to-datetime conversionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Aariq please review solutions in
az_daily()
andaz_hourly()
I just pushed on this branch. I think I figured it out