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

Idea for extract_levels check #262

Closed
wants to merge 1 commit into from
Closed

Conversation

bouweandela
Copy link
Member

This adds an extra check on extract_levels settings

Before you start, please read CONTRIBUTING.md.

Tasks

  • Create an issue to discuss what you are going to do, if you haven't done so already (and add the link at the bottom)
  • Add unit tests
  • Public functions should have a numpy-style docstring so they appear properly in the API documentation. For all other functions a one line docstring is sufficient.
  • If writing a new/modified preprocessor function, please update the documentation
  • Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
  • Codacy code quality checks pass. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.
  • Please use yamllint to check that your YAML files do not contain mistakes
  • If you make backward incompatible changes to the recipe format, make a new pull request in the ESMValTool repository and add the link below

If you need help with any of the tasks above, please do not hesitate to ask by commenting in the issue or pull request.


Closes {Link to corresponding issue}

@mattiarighi mattiarighi added the preprocessor Related to the preprocessor label Sep 19, 2019
)
for coordinate in table_entry.coordinates.values():
if coordinate.axis == 'Z':
break
Copy link
Contributor

Choose a reason for hiding this comment

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

careful here with generic coordinates like olevel or alevel -> are they mapped to eg coord name plev at this stage? Can't remember @jvegasbsc

@schlunma
Copy link
Contributor

@bouweandela do you have any intentions to further work on this for v2.5?

@schlunma
Copy link
Contributor

schlunma commented Feb 2, 2022

I just had another look at this PR and it seems that it's not necessary anymore. We already have a check for the existence of a Z coordinate here (l.1022):

def get_reference_levels(filename, project, dataset, short_name, mip,
frequency, fix_dir):
"""Get level definition from a reference dataset.
Parameters
----------
filename: str
Path to the reference file
project : str
Name of the project
dataset : str
Name of the dataset
short_name : str
Name of the variable
mip : str
Name of the mip table
frequency : str
Time frequency
fix_dir : str
Output directory for fixed data
Returns
-------
list[float]
Raises
------
ValueError:
If the dataset is not defined, the coordinate does not specify any
levels or the string is badly formatted.
"""
filename = fix_file(
file=filename,
short_name=short_name,
project=project,
dataset=dataset,
mip=mip,
output_dir=fix_dir,
)
cubes = load(filename, callback=concatenate_callback)
cubes = fix_metadata(
cubes=cubes,
short_name=short_name,
project=project,
dataset=dataset,
mip=mip,
frequency=frequency,
)
cube = cubes[0]
try:
coord = cube.coord(axis='Z')
except iris.exceptions.CoordinateNotFoundError:
raise ValueError('z-coord not available in {}'.format(filename))
return coord.points.tolist()

Please re-open if necessary.

@schlunma schlunma closed this Feb 2, 2022
@bouweandela
Copy link
Member Author

The advantage of this check was that it does not need to open the file containing the data when parsing the recipe, it checks if the level is in the CMOR table instead. So I think this would still be useful (especially considering #430).

@schlunma
Copy link
Contributor

schlunma commented Feb 2, 2022

Makes sense. As I said, please re-open if necessary. Do you think this is something for v2.5?

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.

4 participants