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

Update NOAA Request function #332

Merged
merged 9 commits into from
Jul 15, 2024
Merged

Conversation

jmcvey3
Copy link
Contributor

@jmcvey3 jmcvey3 commented Jun 24, 2024

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.

@jmcvey3 jmcvey3 requested a review from ssolson June 24, 2024 22:22
…-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
Copy link
Contributor

@ssolson ssolson left a 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?

parameter : str
NOAA paramter (e.g. '' for Discharge, cubic feet per second)
NOAA paramter (e.g. "currents", "salinity", "water_level", "water_temperature",
Copy link
Contributor

Choose a reason for hiding this comment

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

paramter => parameter

@ssolson ssolson added enhancement New feature or request tidal module labels Jul 3, 2024
simmsa and others added 5 commits July 10, 2024 16:08
…#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.
Copy link
Contributor

@ssolson ssolson left a 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.

@ssolson ssolson merged commit b2b903d into MHKiT-Software:develop Jul 15, 2024
39 checks passed
@jmcvey3 jmcvey3 deleted the noaa_request branch July 25, 2024 22:51
@ssolson ssolson mentioned this pull request Aug 12, 2024
ssolson pushed a commit that referenced this pull request Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tidal module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants