-
Notifications
You must be signed in to change notification settings - Fork 100
DistanceTo ancillary file #2131
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
Conversation
* Avoid cubelists of cubelists. * Replace np.product with np.prod. * Replace np.int. * Replace np.NAN with np.nan. * Replace np.NAN with np.nan. * Replace np.NaN with np.nan. * Replace np.product with np.prod. * Replace assertRaisesRegexp with assertRaisesRegex and use collections.abc.Callable instead of collections.Callable. * Simplify test in test_flatten.py to avoid cubelist within a cubelist. * Replace Cube(None) with an alternative. * Unpin environments for testing. * Remove pygam version. * Update improver_a.yml * Remove non-essential dependencies from yml files and add pins. * Minor edits following review comments. * Modify docstring to better reflect the inputs provided.
* Updates to cube_combiner for new environment * Update checksums
maxwhitemet
left a comment
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.
Thanks @mspelman07.
I think the PR is ready to move to second review after these very minor adjustments.
|
Thank you @mspelman07. I am happy with the changes. I'm moving this ticket to second review for @brhooper to have under their profile so they can merge the associated PRs when the Samos feature branch is ready, as you have indicated the need for. |
brhooper
left a comment
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.
Thanks @mspelman07, I've added some minor comments.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2131 +/- ##
==========================================
+ Coverage 98.39% 98.44% +0.05%
==========================================
Files 124 142 +18
Lines 12212 13933 +1721
==========================================
+ Hits 12016 13717 +1701
- Misses 196 216 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
maxwhitemet
left a comment
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.
Thanks @bayliffe. I have suggested some changes to the docstrings. Otherwise, tests pass and I'm happy to approve.
Co-authored-by: Max <[email protected]>
maxwhitemet
left a comment
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.
Happy with the changes made. Approved 👍
maxwhitemet
left a comment
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.
Happy with the changes made. Approved 👍
brhooper
left a comment
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.
Thanks @bayliffe, I've added one comment.
brhooper
left a comment
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.
Thanks for adding the additional test cases @bayliffe. I'm happy to approve this now.
Addresses (https://github.com/metoppv/mo-blue-team/issues/902)
Add in the plugin and unit tests for calculating the Distance To a feature ancillary file. This requires that geopandas is added to the environment and will go into the Samos feature branch when it is created.
Testing: