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

Refactor the functions in derive_evspsblpot due to new iris #2023

Merged
merged 3 commits into from
Feb 12, 2021

Conversation

SarahAlidoost
Copy link
Member

@SarahAlidoost SarahAlidoost commented Feb 12, 2021

Description


Before you get started

Checklist

It is the responsibility of the author to make sure the PR is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.

New or updated recipe/diagnostic:

### New or updated data reformatting script:


To help with the number pull requests:

@SarahAlidoost
Copy link
Member Author

@ESMValGroup/tech-reviewers this PR is ready for review. Who has time for it? Thank you.

@SarahAlidoost SarahAlidoost marked this pull request as ready for review February 12, 2021 14:20
@Peter9192 Peter9192 self-requested a review February 12, 2021 15:01
Copy link
Contributor

@Peter9192 Peter9192 left a comment

Choose a reason for hiding this comment

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

Hey Sarah, nice digging and I guess the workaround is fine for now. However, I think this doesn't improve the overall clarity of the code (ideally the equations can be mapped on to one to those in the paper), so we should consider reverting this once the bug is properly solved in Iris.

I'm surprised that codacy doesn't complain about the import order. Can you verify with isort?

import numpy as np
import iris
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't isort want it the other way around?

Copy link
Member Author

@SarahAlidoost SarahAlidoost Feb 12, 2021

Choose a reason for hiding this comment

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

Thank you. Running prospector on this file didn't complain about it, pylint says: third party import "import numpy as np" should be placed before "import iris"pylint.

@SarahAlidoost
Copy link
Member Author

@ESMValGroup/esmvaltool-coreteam this PR is ready for a merge. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken hydrology recipes due to iris.exceptions.NotYetImplementedError: Coord AuxCoord
3 participants