-
Notifications
You must be signed in to change notification settings - Fork 45
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
Update NOAA Request function #332
Conversation
…-Software#326) * tke updates * Fix shear velocity functions * More detail for tke shear production * Don't rotate heading beyond 360 degrees * Fix some typos in notebook * Rename deprecated function
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.
Overall this looks great James. You improved the docstring and made the function more generalized with minor adjustments.
I don't think I want to add API calls for each parameter. Can you confirm that you tested multiple buoys with different parameter requests to ensure this is all working as expected?
mhkit/tidal/io/noaa.py
Outdated
parameter : str | ||
NOAA paramter (e.g. '' for Discharge, cubic feet per second) | ||
NOAA paramter (e.g. "currents", "salinity", "water_level", "water_temperature", |
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.
paramter => parameter
…#340) * Test: Determine method using input frequency index * Feat: Use sum of sines if ifft is not computable This change allows `surface_elevation` to return a result if the user inputs a spectrum with a frequency index that does not have a zero frequency. If the non zero frequency index condition is found when the method is `ifft` we warn the user and change the method to `sum_of_sines` * Fix: Use previously found frequency index S.index may not exist for some input datasets, but f[0] does and we should use the value of f[0] here. * Test: Warn when using ifft with a non zero frequency * Lint
This PR adds a Github action to test the example notebooks as part of or CD pipeline. Additionally a timeout is added to which notebooks will fail if they exceed the given time.
…into noaa_request
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 made one small change to fix the tidal graphics in the example notebook. I tried multiple parameters and stations and the function worked as expected.
Figured I'd go ahead and solve issue #223 since I have use for it. This PR adds some simple fixes and generalizes error handling.