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

Fix for uas variable of the MCM_UA_1_0 dataset #1102

Merged
merged 8 commits into from
May 11, 2021
Merged

Conversation

remi-kazeroni
Copy link
Contributor

@remi-kazeroni remi-kazeroni commented May 5, 2021

Description

Fix for the 10m height missing for uas

Closes #1101


Before you get started

Checklist

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


To help with the number pull requests:

@remi-kazeroni remi-kazeroni added fix for dataset Related to dataset-specific fix files AR6 Necessary changes for IPCC AR6 labels May 5, 2021
@remi-kazeroni remi-kazeroni added this to the v2.3.0 milestone May 5, 2021
@remi-kazeroni remi-kazeroni self-assigned this May 5, 2021
@valeriupredoi
Copy link
Contributor

@remi-kazeroni and @LisaBock please ping me when you change your various fixes PRs from Draft to RfR - these things are easy to review and I'll merge them very quickly 👍

@remi-kazeroni remi-kazeroni changed the title Fixes for MCM_UA_1_0 dataset Fix for uas variable of the MCM_UA_1_0 dataset May 10, 2021
@remi-kazeroni remi-kazeroni marked this pull request as ready for review May 11, 2021 07:27
@remi-kazeroni remi-kazeroni requested a review from jvegreg as a code owner May 11, 2021 07:27
@remi-kazeroni
Copy link
Contributor Author

@remi-kazeroni and @LisaBock please ping me when you change your various fixes PRs from Draft to RfR - these things are easy to review and I'll merge them very quickly 👍

@valeriupredoi thanks a lot for offering your help for the review of the fixes 👍 We'll mark them RfR asap. This one is ready. Just to let you know, I had a seg fault on circleci with the latest commit but it disappeared simply by rerunning the tests 🍺

@valeriupredoi
Copy link
Contributor

yes, these segfaults are creeping up again, and more alarmingly for the tests that we run in sequential mode, which shouldn't happen, I'll have to look at those again imminently 🍺


Returns
-------
iris.cube.CubeList
iris.cube.Cube
Copy link
Contributor

Choose a reason for hiding this comment

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

you're technically returning a list of one cube, but still a list so the API is incorrect, maybe change with list \n list of [cube]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review @valeriupredoi 👍 Is it fine now? I applied the same suggestion to the tas fix above, is that ok?

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

looks good, cheers @remi-kazeroni 🍺

@valeriupredoi valeriupredoi merged commit 49a2cd1 into master May 11, 2021
@valeriupredoi valeriupredoi deleted the fix_mcm_ua_1_0 branch May 11, 2021 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AR6 Necessary changes for IPCC AR6 fix for dataset Related to dataset-specific fix files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dataset problem: MCM-UA-1-0 has missing 10m height for variable uas
2 participants