Skip to content

updated noresm_to_cmip7_aerosol.yaml#75

Merged
mvertens merged 6 commits into
mainfrom
feature/update_noresm_yaml
Jun 29, 2026
Merged

updated noresm_to_cmip7_aerosol.yaml#75
mvertens merged 6 commits into
mainfrom
feature/update_noresm_yaml

Conversation

@mvertens

@mvertens mvertens commented Jun 29, 2026

Copy link
Copy Markdown
Contributor
  • new norems_to_cmip7_aerosol.yaml
  • changes to get pre-commit tests to pass

@mvertens mvertens requested a review from maritsandstad June 29, 2026 16:47
@mvertens mvertens changed the title updated data/noresm_to_cmip7_aerosol.yaml updated noresm_to_cmip7_aerosol.yaml Jun 29, 2026
@mvertens

Copy link
Copy Markdown
Contributor Author

@maritsandstad - the pre-commit tests now pass.

@maritsandstad maritsandstad left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Have a few minor comments, but happy for this to go in

Comment thread scripts/cmor_driver.py
Comment on lines +756 to +758
from data_request_api.query import data_request as dr
from data_request_api.content import dump_transformation as dt

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm generally not a huge fan of mid-code imports, is there a reason why this import is moved all the way down here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See comment below - this is required for the pre-commit hooks to pass. It's a lazy import.

Comment thread scripts/cmor_driver.py
ml = 1.0 - float(int(ncpus_env) - 1) / 128.0
else:
ml = "auto" # Default memory limit if NCPUS is not set
from dask.distributed import LocalCluster

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same with this, is there a reason why the import has to happen down here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See comment below - you need a lazy import for the pre-commit hooks.

Comment thread scripts/cmor_driver.py
ocn_fx_fields=ocn_fx_fields,
)
futures = client.compute([fut])
from dask.distributed import wait, as_completed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also with this one

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes - it needs to be a lazy import in order for the pre-commit tests to pass. Otherwise - the .github workflow tests fail.

@mvertens mvertens merged commit ee6e257 into main Jun 29, 2026
2 checks passed
@mvertens mvertens deleted the feature/update_noresm_yaml branch June 29, 2026 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants