Skip to content

Fix bug in cf.Field.derivative and cf.relative_vorticity when wrapping with periodic coordinates #290

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.

Sign up for GitHub

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

Jump to bottom
Merged
davidhassell merged 2 commits into from
Dec 23, 2021
Merged

Fix bug in cf.Field.derivative and cf.relative_vorticity when wrapping with periodic coordinates #290

davidhassell merged 2 commits into from
Dec 23, 2021

Conversation

@davidhassell
Copy link
Collaborator

@davidhassell davidhassell commented Dec 23, 2021

Fixes #289

sadielbartholomew
sadielbartholomew approved these changes Dec 23, 2021
View reviewed changes
Copy link
Member

@sadielbartholomew sadielbartholomew left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great: fixes the issue at hand and covers the case with new testing. The CI failures are again due to the test_file.nc not being generated or recognised (yet to solve that one). (I've made one minor suggestion but it only relates to a commenting typo so feel free to ignore if simpler to merge as-is.)

@davidhassell @sadielbartholomew
Typo
9b04163 
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
@davidhassell
Copy link
Collaborator Author

davidhassell commented Dec 23, 2021

Thanks, Sadie, for the quick review. I'll merge, now.

@davidhassell davidhassell merged commit f18eaed into NCAS-CMS:master Dec 23, 2021
@sadielbartholomew
Copy link
Member

sadielbartholomew commented Dec 23, 2021

@davidhassell, apologies since this only came to my attention now due to #291, but there appears to be a test failure related to the changes made in this PR that appears on multiple Actions CI workflows, though it doesn't fail locally (not sure why there is the difference and don't have time to investigate pre-Christmas): see #291 (comment). If you could investigate why that failure has emerged after this PR, any insight would be helpful. Thanks.

@davidhassell davidhassell mentioned this pull request Dec 24, 2021
Merged
@davidhassell
Copy link
Collaborator Author

davidhassell commented Dec 24, 2021

b77e5a2 should fix this for now. The whole vorticity thing needs overhauling, so it's not worth doing a more thorough job at this time.

@davidhassell davidhassell added this to the 3.12.0 milestone Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sadielbartholomew sadielbartholomew sadielbartholomew approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.12.0

Development

Successfully merging this pull request may close these issues.

Error in cf.Field.derivative and cf.relative_vorticity when wrapping with periodic coordinates

2 participants

@davidhassell @sadielbartholomew