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

Iris module error when running recipe_ecs_constraints.yml #1380

Closed
remi-kazeroni opened this issue Nov 2, 2021 · 8 comments · Fixed by #1382
Closed

Iris module error when running recipe_ecs_constraints.yml #1380

remi-kazeroni opened this issue Nov 2, 2021 · 8 comments · Fixed by #1382
Assignees
Labels
bug Something isn't working iris Related to the Iris package release

Comments

@remi-kazeroni
Copy link
Contributor

Describe the bug
I re-tested the recipe_ecs_constraints.yml for the release (#2354) after the provenance issue was fixed and got the following error in the diagnostics diag_x_volodin_cmip5/ecs_predictor:

Traceback (most recent call last):
  File "/mnt/lustre01/pf/b/b309192/ESMValTool/esmvaltool/diag_scripts/emergent_constraints/ecs_scatter.py", line 52, in <module>
    from esmvalcore.cmor.fixes import add_plev_from_altitude, add_sigma_factory
  File "/mnt/lustre01/pf/b/b309192/ESMValCore/esmvalcore/cmor/fixes.py", line 3, in <module>
    from ._fixes.shared import (
  File "/mnt/lustre01/pf/b/b309192/ESMValCore/esmvalcore/cmor/_fixes/shared.py", line 18, in <module>
    class AtmosphereSigmaFactory(iris.aux_factory.AuxCoordFactory):
AttributeError: module 'iris' has no attribute 'aux_factory'

Not sure why this is surfacing now...

@remi-kazeroni remi-kazeroni added bug Something isn't working iris Related to the Iris package release labels Nov 2, 2021
@valeriupredoi
Copy link
Contributor

that needs to be imported inside the diagnostic as a submodule, just doing an:

import iris
import iris.aux_factory

will solve it. The cause is iris adopting isort in 3.1.0

@schlunma
Copy link
Contributor

schlunma commented Nov 2, 2021

Apparently that's something that has been changed in iris 3.1...My environment still had version 3.0.1. I will open a PR with the changes. By this link here I'm amazed that nothing else in the core broke 😅 https://scitools-iris.readthedocs.io/en/latest/whatsnew/3.1.html#incompatible-changes

@valeriupredoi
Copy link
Contributor

quite a few things have indeed broken (including individual runs of pytest tests), but we fixed most everything and the fact that the import of the main package (iris) is in the __init__ headers helps 😁

@schlunma
Copy link
Contributor

schlunma commented Nov 2, 2021

Simply adding an import iris.aux_factory does not fix this recipe. Since AtmosphereSigmaFactory has been added to iris in 3.1 (SciTools/iris#4052) the recipe fails now with a duplicate air_pressure coordinate:

iris.exceptions.CoordinateNotFoundError: 'Expected to find exactly 1 coordinate, but found 2. They were: air_pressure, air_pressure.'

For this I will need to remove the custom version of AtmosphereSigmaFactory which we have in esmvalcore/cmor/_fixes/shared.py. I can try to get this fixed by the end of the week, but these changes will probably be too extensive for 2.4. I propose to postpone this for a later (bugfix) release. @zklaus @valeriupredoi would that be okay for you?

@schlunma
Copy link
Contributor

schlunma commented Nov 3, 2021

I opened two (draft) PRs that fix the problem: #1382 and ESMValGroup/ESMValTool#2405

@remi-kazeroni
Copy link
Contributor Author

I opened two (draft) PRs that fix the problem: #1382 and ESMValGroup/ESMValTool#2405

Thanks for doing so @schlunma! Shall I start reviewing those? Or do you plan to put some more work before marking the PRs as ready for review?

@zklaus
Copy link

zklaus commented Nov 3, 2021

I think the main reason @schlunma marked them as a draft, for now, is to keep them until after the release. However, since we are already pinning iris >= 3.1.0, this is an actual clash between that version of iris and our own code, and this seems clean enough, I would actually be ok with getting this in the release.

So, if you, @remi-kazeroni, could provide a review, and if you, @schlunma, are ok with this, I'd like to get the Core PR in asap, cut a new rc, then get the Tool PR in and have things ready for a final round of testing at the end of this week, hopefully starting tomorrow.

Sound good?

@schlunma
Copy link
Contributor

schlunma commented Nov 3, 2021

Sound good, the PR is ready for review! Thanks @zklaus and @remi-kazeroni!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working iris Related to the Iris package release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants