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.

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

Added AtmosphereSigmaFactory #4052

Merged
merged 6 commits into from
Aug 16, 2021
Merged

Added AtmosphereSigmaFactory #4052

merged 6 commits into from
Aug 16, 2021

Conversation

schlunma
Copy link
Contributor

@schlunma schlunma commented Mar 5, 2021

🚀 Pull Request

Description

iris.aux_factory does not provide an aux factory for atmosphere_sigma_coordinate yet. This PR adds the class AtmosphereSigmaFactory similar to iris.aux_factory.OceanSigmaFactory. Most of the code for the new class is taken from this. Right now, we have a custom AtmosphereSigmaFactory implemented into ESMValTool. This works nicely, but needs to be added manually to the cube after loading the file.

Closes #4051.

Note: The "What's new" is still missing, I will add that later!

Thanks!

Consult Iris pull request check list

@SciTools-assistant SciTools-assistant added the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Mar 5, 2021
@schlunma
Copy link
Contributor Author

schlunma commented Mar 5, 2021

Just signed the CLA 👍

@bjlittle bjlittle self-assigned this Mar 5, 2021
@SciTools-assistant SciTools-assistant removed the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Mar 5, 2021
@bjlittle bjlittle self-requested a review March 5, 2021 18:05
@schlunma schlunma mentioned this pull request Apr 1, 2021
Closed
@pp-mo
Copy link
Member

pp-mo commented Jul 30, 2021

Hi @schlunma
Sorry for such a long delay on this, we have been rather short of time + pulled in all directions !
I see it now needs at least a(nother) rebase -- sorry about that.
Can we see about getting this finished now, within next 2-3 weeks ?
If so, we can get it into Iris 3.1.0, which would be great.

@schlunma
Copy link
Contributor Author

schlunma commented Aug 4, 2021

Hi @pp-mo, thanks! I will try to get the rebase done as quickly as possible, just had a quick look and saw that this probably requires lots of changes with pyke gone.

@schlunma
Copy link
Contributor Author

schlunma commented Aug 5, 2021

This is ready for review now! Required less changes than expected 👍

@bjlittle bjlittle removed their assignment Aug 9, 2021
@lbdreyer lbdreyer self-assigned this Aug 10, 2021
lbdreyer
lbdreyer requested changes Aug 12, 2021
View reviewed changes
Copy link
Member

@lbdreyer lbdreyer left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @schlunma, and thank you for updating it to work with the replacement of pyke! This is a great addition!

I have a few comments. Once those are addressed we can hopefully get this in in time for Iris 3.1. We are hoping to cut the release candidate at the start of next week. If you think that may be too little time, please let me know and we can try to sort something out!

lib/iris/aux_factory.py Outdated Show resolved Hide resolved
lib/iris/aux_factory.py Outdated Show resolved Hide resolved
lib/iris/aux_factory.py Outdated Show resolved Hide resolved
lib/iris/aux_factory.py Outdated Show resolved Hide resolved
lib/iris/tests/unit/aux_factory/test_AtmosphereSigmaFactory.py Outdated Show resolved Hide resolved
lib/iris/tests/unit/aux_factory/test_AtmosphereSigmaFactory.py Outdated Show resolved Hide resolved
@schlunma
Copy link
Contributor Author

Thanks @lbdreyer for your review. I tried to address all your comments; let me know if you need anything else!

lbdreyer
lbdreyer approved these changes Aug 16, 2021
View reviewed changes
Copy link
Member

@lbdreyer lbdreyer left a comment

Choose a reason for hiding this comment

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

Brilliant! Thanks @schlunma for responding so promptly!

@lbdreyer lbdreyer merged commit 303902d into SciTools:main Aug 16, 2021
tkknight added a commit to tkknight/iris that referenced this pull request Sep 6, 2021
@tkknight
Merge branch 'main' into docs_checklist_fix
112111d 
* main:
  [pre-commit.ci] pre-commit autoupdate (SciTools#4299)
  Updated environment lockfiles (SciTools#4298)
  cartopy feature download (SciTools#4304)
  reset whatsnew latest (SciTools#4288)
  Updated environment lockfiles (SciTools#4289)
  Update cube.py (SciTools#4287)
  Missing whatsnew entries for 3.1 release. (SciTools#4283)
  Update CF standard name table to v77 (SciTools#4282)
  Added AtmosphereSigmaFactory (SciTools#4052)
  Updated environment lockfiles (SciTools#4281)
This was referenced Nov 2, 2021
Closed
Merged
@schlunma schlunma deleted the atmosphere_sigma_factory branch December 20, 2021 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lbdreyer lbdreyer lbdreyer approved these changes

@bjlittle bjlittle Awaiting requested review from bjlittle

Assignees

@lbdreyer lbdreyer

Labels
Type: Enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support of AtmosphereSigmaFactory
5 participants
@schlunma @pp-mo @lbdreyer @bjlittle @SciTools-assistant