-
Notifications
You must be signed in to change notification settings - Fork 84
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
add to man page for coops_search #213
Comments
what is the coops.html page ? yes, can definitely document this. do you know where the NOAA docs state this? that block of code looks pretty good. pull request? |
coops.html is the man page generated from rnoaa/man/coops.Rd The NOAA API docs mentions this in a table near the bottom ( (https://tidesandcurrents.noaa.gov/api/#maximum ) If you submit a url with a long duration, datagetter returns an error message:
For example: Note that the limit differs by product, but interval=h in the API has the potential to change it for many of the products. If your ... only passes parameters to curl (and not interval= to the API), the duration limit is 1 year for hourly_height and high_low, and 10 years for daily_mean and monthly_mean, and I don't think is applicable to datums. If I have time this weekend I'll fork the repository & extend my trapping code fragment to test durations against the product-specific maxima. If I can't this weekend, I should be able to get at it the following weekend. I won't feel insulted if you do the patch instead of waiting for me, but I also don't think it is an emergency, especially if you can add a brief note to coops.Rd in the meantime. In the broader picture, I haven't decided whether it would be better to put my code for splitting long requests into 31 day chunks inside your functions, or leaving it as an external wrapper function that chunks the dates, calls your functions, then assembles the results. In terms of (you) maintaining your codebase in the face of changing APIs, I suspect an external wrapper might be better. What are your thoughts? Would you be interested in a function by_chunk(begin_date, end_date, chunk_size, rnoaa_fun, ...) contributed to your package, or should I just put it in my own repository of utility functions? |
ah, okay, the man page for coops. thanks for the link
I'll add the note to man page, and let you do the code change - next milestone with issues is here https://github.com/ropensci/rnoaa/milestone/10 - want to submit to CRAN relatively soon, within a week or so, but deadline to go later
I'd vote for putting it in here. If you want that feature i'm guessing others do as well. agree it's best not to break package APIs, but we could add an additional function or parameter inside the current function. i think probably a second fxn is best so we don't break behavior of the current fxn |
Scott-- |
I agree best as a separate function. |
i think your PR sorted this out, clsoing, reopen if not |
Could you add to the coops.html page a note that for most if not all data products, the NOAA coops API only vends up to 31 days of data at a time?
works, but:
fails with a misleading (but correct) error message
Also, you might add a trap for this circa line 138 (not sure if this is in your exact style):
At least for now I suggest adding a note to the documentation and possibly the error trapping. Adding the ability to have end_date - start_date >31 days is not simple. Generating a list of 31-day requests from start_date and end_date is easy; the returned data for each chunk can be clean or completely missing or at least 1 other pathology. If I get to the point where I'm confident I understand all of the possibilities, I may make a pull request and submit proposed code.
The text was updated successfully, but these errors were encountered: