This repository has been archived by the owner on Sep 30, 2024. It is now read-only.
Wind Diagnostics API #53
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Just a basic test to get things rolling, all we do is assert that the endpoint responds with a 200 (which it fails, because the endpoint doesn't exist yet).
I think I like having access to everything in the mock submodule and having it namespaced so we call mock.patch instead of just patch.
I created a dataclass to represent diagnostic data for vector data so that we can benefit from more type checking. This means the responsibility for converting the dataclass into JSON is the responsibility of the API. Right now the diag.wind function doesn't do anything other than return an empty VectorDiag object.
Following the same pattern we used for temperature (which needs to be refactored), diag.wind takes a minimization loop as an argument and the route calls it twice to retrieve the guess and analysis loops to build the response.
I think it makes more sense to send wind data as direction and magnitude rather than u, v vectors, since most of what we'll be visualizaing is direction and magnitude. I'm also going to send the obs and forecast values, instead of obs - forecast, since some of the vector visualization will actually show both vectors, not just the difference. It might be a good idea to use a similar format for scalar values, since it gives us more flexibility; we'll have to see how large these responses are.
I think having patch namespaced in the mock module help readability, plus it makes it easier to get to other things from the mock module, like call or create_autospec if needed.
I've tested the happy path for the diag.wind function to ensure that it requests the correct diagnostic data and uses the correct dimensions of the Dataset to properly construct the return value. There's a lot of mocking going on in here, and it makes me worried that these tests are fragile, but it does fully isolate this test from its collaborators.
Switch to using pytest's built in tmp_path fixture for defining the DIAG_DIR config variable. This way we have a temporary directory where we can write NetCDF files for use in testing.
I've added a function that will take the variable and minimization loop as parameters and return a Dataset read from the corresponding NetCDF file. I'll need to refactor get_diagnostic into something that is specific to fetching temperature and makes use of this function. I have two placeholder tests for handling missing or bad input files for open_diagnostic.
Raise a FileNotFoundError in open_diagnostic if the file does not exist.
This might be overkill, but I think it might be worth documenting what the expected message is just so that we know what to look for in logs.
I don't think we want the diag.wind method to handle exceptions from diag.open_diagnostic, this way the routes can decide how best to handle errors in the request with appropriate HTTP status codes.
Since we're letting exceptions bubble up, we can guarantee that if a function returns, it has a value.
Respond with a 404 if the diagnostics files can't be found and a 500 if the files can't be read.
I don't want to forget to implement this. I should've added this test when I stubbed the method.
Calculate direction and magnitude of a vector from the (u, v) vectors we have in our diag files and create a new VectorVariable object from them. I've checked my test cases against from the NCL documentation: http://ncl.ucar.edu/Document/Functions/Contributed/wind_direction.shtml The only difference is that we end up reporting a direction of 90° for a vector of (0, 0) instead of a direction of 0°. This may end up mattering, but I think it will be ok, since we'll be internally consistent when we calculate the Obs - Fcst.
Rather than dealing with np.arctan2's 0.0 vs -0.0 silliness, We go through and set the direction for all vectors with a magnitude of 0 to 0°. This is consistent with the NCL wind_function.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Define the API for fetching wind speed and direction diagnostics.