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

Revert multimodel module to v2.2.0 #1202

Closed
wants to merge 7 commits into from
Closed

Conversation

valeriupredoi
Copy link
Contributor

Description

DO NOT MERGE JUST YET - I need to create a clone of current main so we don't lose the current MM module

Closes #1201


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:

@valeriupredoi valeriupredoi added the preprocessor Related to the preprocessor label Jun 29, 2021
Copy link

@zklaus zklaus left a comment

Choose a reason for hiding this comment

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

It's not decided to revert this module just yet.

@valeriupredoi
Copy link
Contributor Author

It's not decided to revert this module just yet.

indeed, we need to reach a decision on this, I suggest we do it soon so we know which way we go 👍

@codecov
Copy link

codecov bot commented Jun 29, 2021

Codecov Report

Merging #1202 (274305c) into main (3b653c4) will decrease coverage by 0.01%.
The diff coverage is 92.48%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1202      +/-   ##
==========================================
- Coverage   85.51%   85.50%   -0.02%     
==========================================
  Files         188      188              
  Lines        9147     9161      +14     
==========================================
+ Hits         7822     7833      +11     
- Misses       1325     1328       +3     
Impacted Files Coverage Δ
esmvalcore/_recipe.py 91.10% <ø> (-0.02%) ⬇️
esmvalcore/_recipe_checks.py 81.98% <ø> (-0.23%) ⬇️
esmvalcore/preprocessor/_multimodel.py 92.70% <92.48%> (-1.41%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b653c4...274305c. Read the comment docs.

@valeriupredoi
Copy link
Contributor Author

heads up we should close this if no revert is decided to be needed @zklaus

@zklaus zklaus added this to the v2.3.1 milestone Jul 7, 2021
@zklaus
Copy link

zklaus commented Jul 7, 2021

Cheers for the heads-up, V. I have it on the radar (now the milestone as well). Revert is still a possibility imho until the ancillary variables issue is resolved, so let's keep it open for now. Having it on the 2.3.1 milestone makes sure we don't just forget it.

@zklaus
Copy link

zklaus commented Jul 23, 2021

Issues dealt with elsewhere, see ESMValGroup/ESMValTool#2218.

@zklaus zklaus closed this Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preprocessor Related to the preprocessor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New multimodel module can be slow and buggy for certain recipes
2 participants