-
Notifications
You must be signed in to change notification settings - Fork 86
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
Adds masked_add to cube combiner #2015
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2015 +/- ##
==========================================
- Coverage 98.39% 98.33% -0.07%
==========================================
Files 124 130 +6
Lines 12212 12636 +424
==========================================
+ Hits 12016 12425 +409
- Misses 196 211 +15 ☔ View full report in Codecov by Sentry. |
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 have looked through these tests and I think they seem fine. I have run the unit tests and they pass, so I'm happy with this 👍
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 is good. Here's a few suggestions to make it simpler and to add missing documentation.
…distance * refs/heads/master: Height of max (metoppv#2020) Migrate CLI functionality: simple_bias_correction (metoppv#2018) BUG: Fix Scheduled Tests Sphinx-Pytest-Coverage (metoppv#2021) Plugin discovery (metoppv#2009) Adds masked_add to cube combiner (metoppv#2015) Apply mask to cube (metoppv#2014) Make difference module handle circular cubes (metoppv#2016) # Conflicts: # improver/utilities/spatial.py # improver_tests/utilities/spatial/test_DifferenceBetweenAdjacentGridSquares.py
* Adds masked_add to cube combiner * Remove unnecessary import * formatting * add docstrings and simplifications * formatting --------- Co-authored-by: Marcus Spelman <[email protected]>
…distance * refs/heads/master: Height of max (metoppv#2020) Migrate CLI functionality: simple_bias_correction (metoppv#2018) BUG: Fix Scheduled Tests Sphinx-Pytest-Coverage (metoppv#2021) Plugin discovery (metoppv#2009) Adds masked_add to cube combiner (metoppv#2015) Apply mask to cube (metoppv#2014) Make difference module handle circular cubes (metoppv#2016) # Conflicts: # improver/utilities/spatial.py # improver_tests/utilities/spatial/test_DifferenceBetweenAdjacentGridSquares.py
* Adds masked_add to cube combiner * Remove unnecessary import * formatting * add docstrings and simplifications * formatting --------- Co-authored-by: Marcus Spelman <[email protected]>
…distance * refs/heads/master: Height of max (metoppv#2020) Migrate CLI functionality: simple_bias_correction (metoppv#2018) BUG: Fix Scheduled Tests Sphinx-Pytest-Coverage (metoppv#2021) Plugin discovery (metoppv#2009) Adds masked_add to cube combiner (metoppv#2015) Apply mask to cube (metoppv#2014) Make difference module handle circular cubes (metoppv#2016) # Conflicts: # improver/utilities/spatial.py # improver_tests/utilities/spatial/test_DifferenceBetweenAdjacentGridSquares.py
* Added test for equal-area grids and got it passing Made input test fixture configurable Got test for gradient on lat/long grid failing for correct reasons (response is in m/s/degree; needs to be /s). Commented out regridding test for now. Hacked about and more or less got latlon distance calculation working but made a lot of mess. Plan to abstract it to its own class so I can test it properly and allow it to be reused. First test for distance class fails in the right way Got happy path test passing Added more tests. Did some tidying Prepared to implement equalarea logic. Wrote failing test. Test for equalarea passes Tidying up Gradient test passes Improved tests. Nearly working. Problem with generating expected data for y when interpolated back out to original dims. Added extra test cases and got all tests working. Reduced required absolute precision due to unexplained weirdness in how iris linear interpolation works when regridding with large gradients. Tidied up tests Ran black against test file then re-squared the arrays Tidying Got non-standard units working for equal area projections. Not going to implement for latlong projections, as anything other than degrees is rare, and will not fail silently in any case Removed unused test Moderately graceless exception handling. Open to suggestions for a better way to work out which coordinates represent latitude and longitude. Fixed cube instantiation to allow better way of checking projection type More tidying Tidied imports in spatial utilities Review comments - Tuple type hinting fix Ran autoformatting tools Added docstrings for some methods Moved static method back to original location Added docstrings to all of the new functions in spatial.py Added some more docstrings to test helper functions Review actions around error handling Numpy efficiency improvement Review actions and tidying Added extra tests and made distance module handle all coordinate systems that provide distances between grid points Added a todo comment. Will be actioned shortly spelling correction Co-authored-by: Stephen Moseley <[email protected]> more spelling corrections Co-authored-by: Stephen Moseley <[email protected]> Yet more spelling corrections Co-authored-by: Stephen Moseley <[email protected]> And another Co-authored-by: Stephen Moseley <[email protected]> Spelling corrections: Meters -> Metres Switched to using earth radius supplied with input cube rather than hard-coded constant Added docstrings to distance tests Review actions Co-authored-by: Stephen Moseley <[email protected]> tidied up y latlon distances method Tidied up x distances calculation Arguably simplified latlon distance methods. Going to try to split out further using factory pattern. Will revert if it just makes things worse. Tests passing after factory refactor. Still needs lots of tidying. Finished tidying A little more tidying Review actoin Updated dosctring and deleted corresponding one on child class as sphinx handles docstring inheritence Fixed gradient module broken by refactor of distance module Docstring change. Ran autoformatting Added appropriate decoration to distances base class Added failing tests to gradient module for wrapping around the meridian - still need to move grid points used in tests to cover whole globe at 120 degree increments. Made test cube generation work with non-equal x and y spacing Update improver/utilities/spatial.py Co-authored-by: Stephen Moseley <[email protected]> Docstring improvements Working on gradient tests. They seem to only work for 10 degrees separations. Expected values are correct, actuals wrong. Amended test data to give gradients large enough that no tests pass due to both expected and actual being too small to tell the difference between them Fixed bug causing tests to fail for non wrap-around case. Spelling corrections Updated docstring Switched to 120 degree longitude separations. Some tests now failing. Expected values correct; mid way through debugging actuals. Got part way to testing wrap-around interpolaiton. Found a bug with non wrap-around calculations for large angles along the way, and got part way through fixing it Added tests for different longitude separations to distance module and fixed some bugs. Added failing tests to Gradient module for circular latlong grid More faffing with tests Amalgamated some distance tests into single parameterised function. Fixed testing bug More test tidying Distance tests fail correctly for circular cubes Started work to make distance caluclation work for circular coords Removed dependency on diffs from projection cube subclass. Did some tidying. Made gradient stop passing diffs to distances method Added test for projected cubes which wrap around the meridian Made difference module handle cubes that wrap around the meridian Added another failing test for difference module. Differences calculated correctly but need to add extra dimension for the wrap-around distance. Driveby docstring fix Removed some completed todos. Added a couple of new ones Removed another completed tod Got distance module passing test for circualar coords Make difference module handle circular cubes (#2016) * Resolved merge conflict * Added another failing test for difference module. Differences calculated correctly but need to add extra dimension for the wrap-around distance. * Resolved merge conflicts * Auto-formatting * Import Bugfix * Ran isort against spatial.py * Added test for appropriate error for unhandled cubes and made code match it. * iSort on spatial.py again * Another attempt at autoformatting * Tidied up * Removed confusing variable name * Autoformatting * Tried manually sorting import order as isort still failing in github actions despite running locally and pushing * And again * And again * Linting suggestion * Review Actions * Ran isort to fix gha * Added test for cubes with flipped axes (lonitude increasing along columns not rows and vice versa for latitude). Fixed code to handle this case. * black, isort, flake8 * Ran isort in isolation * Fixed test failures resulting from breaking changes made by upstream PR * Black * updates as per review comments and adds test for 0-360 degree case * update comment * formatting --------- Co-authored-by: Marcus Spelman <[email protected]> * Removes duplicate unit test that has been superseded. * Fixes calculation of x coord for distances cube with circular coord. * Updates licence in new file. * Updates units from "metres" to "m". * Refactors tests for utilities/spatial into a subdir as per convention. * Improves doc-strings * Deals with todos: - circular projections raise error - make_test_cube uses set_up_variable_cube - black * Deals with todos: - Tests non-uniform lat-lon grids * Deals with todos: - No more "full haversine" refs to delete * Deals with todos: - Equalarea circular grid with DifferenceBetweenAdjacentGridSquares * Deals with todos: - Moves one into a doc-string - Adds another about unit test complexity * Style * Style * Style * Codecov - removes `pass` from abstract methods as these can never be reached and are unnecessary. * Adds test for uncovered raises statement * Simplifies tests for GradientBetweenAdjacentGridSquares which led to: - Simplification of calculation of coordinate points - Always raise error for circular projections - Output cubes inherit metadata from input cube * Adds doc-string and strengthens a test * Fixes handling of mandatory attributes * Style * Adds metadata and error tests for regrid=True mode * Adds data tests for regrid=True mode * Adjusts test data so that we can see obvious expected behaviour with circular data. * Removes redundant precision specifier * Corrects doc-string * Corrects metadata expected outputs * Fixes test that only fails in some environments. * Moves cube argument to process method for consistency with other plugin behaviour. * Adds doc-string * Improves doc-string * Removes todo comment * Updates doc-strings following review. * Updates tests following review * Make difference module handle circular cubes (#2016) * Resolved merge conflict * Added another failing test for difference module. Differences calculated correctly but need to add extra dimension for the wrap-around distance. * Resolved merge conflicts * Auto-formatting * Import Bugfix * Ran isort against spatial.py * Added test for appropriate error for unhandled cubes and made code match it. * iSort on spatial.py again * Another attempt at autoformatting * Tidied up * Removed confusing variable name * Autoformatting * Tried manually sorting import order as isort still failing in github actions despite running locally and pushing * And again * And again * Linting suggestion * Review Actions * Ran isort to fix gha * Added test for cubes with flipped axes (lonitude increasing along columns not rows and vice versa for latitude). Fixed code to handle this case. * black, isort, flake8 * Ran isort in isolation * Fixed test failures resulting from breaking changes made by upstream PR * Black * updates as per review comments and adds test for 0-360 degree case * update comment * formatting --------- Co-authored-by: Marcus Spelman <[email protected]> * Apply mask to cube (#2014) * Add apply_mask cli and function * remove print statement * Updates to make sure plugin doesn't require cubelist input * Update Docstring --------- Co-authored-by: Marcus Spelman <[email protected]> * Adds masked_add to cube combiner (#2015) * Adds masked_add to cube combiner * Remove unnecessary import * formatting * add docstrings and simplifications * formatting --------- Co-authored-by: Marcus Spelman <[email protected]> * Plugin discovery (#2009) * BUG: Fix Scheduled Tests Sphinx-Pytest-Coverage (#2021) Pulling over pinned packages introduced to the `conda-forge.yml` into the `latest.yml` file. #2011 * Migrate CLI functionality: simple_bias_correction (#2018) * refactored simple_bias_correction cli * docstring and contributing * changes for actions * sphinx building locally without error * whitespace change * bugfix: test upper_bound * changes based on review * review changes * review - doc-strings * Height of max (#2020) * Plugin for calculating the height of the maximum vertical velocity once the maximum vertical velocity has been calculated. * Minor code updates. * Deleting or adding required spaces. * Updating some comments. * Adds masked_add to cube combiner * Remove unnecessary import * formatting * Updating and improving unit tests, and adding cli. * Isort changes. * Formatting changes. * Sorting out trailing whitespaces. * Sorting out formatting. * flake8 change. * Further changes to get acceptance tests to work. * Changes to get acceptance test data to run. * add docstrings and simplifications * formatting * update checksums and correct acceptance test bug * Update default name and change low_or_high to boolean * updates acceptance tests * formatting --------- Co-authored-by: Kathryn.Howard <[email protected]> Co-authored-by: Marcus Spelman <[email protected]> * Removes differences in spatial.py w.r.t. reviewed version * Resolves differences with reviewed version * Make difference module handle circular cubes (#2016) * Resolved merge conflict * Added another failing test for difference module. Differences calculated correctly but need to add extra dimension for the wrap-around distance. * Resolved merge conflicts * Auto-formatting * Import Bugfix * Ran isort against spatial.py * Added test for appropriate error for unhandled cubes and made code match it. * iSort on spatial.py again * Another attempt at autoformatting * Tidied up * Removed confusing variable name * Autoformatting * Tried manually sorting import order as isort still failing in github actions despite running locally and pushing * And again * And again * Linting suggestion * Review Actions * Ran isort to fix gha * Added test for cubes with flipped axes (lonitude increasing along columns not rows and vice versa for latitude). Fixed code to handle this case. * black, isort, flake8 * Ran isort in isolation * Fixed test failures resulting from breaking changes made by upstream PR * Black * updates as per review comments and adds test for 0-360 degree case * update comment * formatting --------- Co-authored-by: Marcus Spelman <[email protected]> * Resolves differences with reviewed version again --------- Co-authored-by: Marcus Spelman <[email protected]> Co-authored-by: mspelman07 <[email protected]> Co-authored-by: Carwyn Pelley <[email protected]> Co-authored-by: SamGriffithsMO <[email protected]> Co-authored-by: Kathryn.Howard <[email protected]>
Addresses: https://metoffice.atlassian.net/browse/EPPT-1305
Adds the masked_add option to cube combiner. This will add together the cubes but treat masked points as equal to 0.
Testing: