-
Notifications
You must be signed in to change notification settings - Fork 94
Continuous #832
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
Continuous #832
Conversation
|
Hey there, I have time to review Wed/Thur. |
R/read_waterdata_continuous.R
Outdated
| #' @export | ||
| #' @param monitoring_location_id `r get_params("continuous")$monitoring_location_id` | ||
| #' @param parameter_code `r get_params("continuous")$parameter_code` | ||
| #' @param statistic_id `r get_params("continuous")$statistic_id` |
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.
Note to self to test: is this really a valid input? Isn't the stat id for continuous 00011?
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.
I think it's a valid input, but only 00011 will return data. We could take it out of the argument list so hopefully it's less confusing? I'll double check there aren't some weird random other stat ids for continuous data.
Co-authored-by: Elise Hinman <121896266+ehinman@users.noreply.github.com>
Co-authored-by: Elise Hinman <121896266+ehinman@users.noreply.github.com>
| #' | ||
| #' @description `r get_description("continuous")` | ||
| #' | ||
| #' Currently, the services only allow up to 3 years of data to be requested with |
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.
Is this a hard and fast rule?
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.
Edit: asked Mike, got the answer. This is a helpful piece of info!
|
This test produced a warning and the properties argument didn't work: It returns the requested data, just with all the columns. |
|
Good catch, hard to order on a column that doesn't exist. |
Co-authored-by: Elise Hinman <121896266+ehinman@users.noreply.github.com>
vignettes/join_by_closest.Rmd
Outdated
| knitr::kable(head(uv_trim)) | ||
| ``` | ||
|
|
||
| Next we'll clean up the discrete water quality "qw" data to make it easy to follow in this tutorial. |
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.
Do we need to keep using "qw" when its original source doesn't exist anymore? I kinda pre-date "qw" so find it kind of a weird abbreviation.
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.
What's wrong with the quality of water measured at our gage?? 😂(I'll change it!)
| Once the pipeline has completed, you can load the `ohio_discharge` data frame into your environment with `tar_load`: | ||
|
|
||
| ```{r eval=FALSE} | ||
| tar_load(ohio_discharge) |
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.
SNAZZY. I just tested this out and it worked great.
vignettes/tutorial.Rmd
Outdated
| * USGS APIs (Water Data) | ||
| * Water Quality Portal (WQP) | ||
| * USGS APIs (Water Data), which are new or in-development . | ||
| * National Water Information System (NWIS) - Legacy system that will eventually be retired. |
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.
Double-checking with Mike...but I believe I remember him saying that while Water Services is going away, the collective "NWIS" will still be a term used in this ecosystem. @mikemahoney218-usgs do I have that right?
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.
The challenge specifically for dataRetrieval is that we called all of those functions (whether it was waterservices or waterdata) NWIS (readNWISxxx). I'll do a little effort to make it clear that NWIS Water Services are going to be retired - but for most dataRetrieval users, that distinction will not matter. We want to be clear that all the NWIS dataRetrieval functions will stop working eventually.
Co-authored-by: Elise Hinman <121896266+ehinman@users.noreply.github.com>
ehinman
left a comment
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.
This looks good to me, @ldecicco-USGS. Thanks for the opportunity to review. Approved.
No description provided.