Cmorise cci lst v3 #3656
Cmorise cci lst v3 #3656
Conversation
…nts values, can be viewed in log file
…ashing the whole run
There was a problem hiding this comment.
many thanks for your work @morobking 🍺 And my apologies for the review tardiness, summer holidays just over! I have a few minor points, and a couple rather important one related to your use of try and except in some places, please have a look and tell me if things are not clear 👍
| # loop_date += relativedelta.relativedelta(months=1) | ||
| # else: | ||
| # logger.info('%d: no data found', loop_date.year) | ||
| # loop_date += relativedelta.relativedelta(years=1) |
There was a problem hiding this comment.
it'd be useful to either remove commented out code, if you know for sure it's not needed no more, or place in a callable function/object so that it can be used at a later stage
|
|
||
| Modification history | ||
| 20201015 Started by Robert King | ||
| 20201029 Day/Night averaging added along with CMOR utils |
There was a problem hiding this comment.
could you please keep this header and update it with current info? All other cmorizers have it, and it's a useful bit of doc
|
|
||
| from esmvaltool.cmorizers.data import utilities as utils | ||
| from ...utilities import fix_coords |
There was a problem hiding this comment.
package-level imports are better than relative path imports, so here it'd be from esmvaltool.cmorizers.data.utilities import fix_coords
| month, | ||
| vals['raw'], | ||
| ) | ||
| except: |
There was a problem hiding this comment.
blank excepts are bad, do you know what sort of exception the iris cube loader may throw at you? It's not clear from the info printed below
| try: | ||
| cubes.remove_coord('time') | ||
| except: | ||
| logger.info('Coord fix issue %s' % cubes.long_name) |
There was a problem hiding this comment.
here too - I'd be much happier if you caught the right exception eg coord not found etc; to the very least please be more specific in your logger message - it contains very little info about what went wrong. If you are unsure of the type and handling of exception, it's always better to leave it blow up, and not try catch it, mask it, then output an non-descriptive message
| try: | ||
| cubes.coords()[2].standard_name = 'longitude' | ||
| except: | ||
| # No change needed | ||
| pass |
There was a problem hiding this comment.
what's the point of this? The code doesn't fail unless specifically instructed, and if it doesn't fail, and no warning or info is fed to the user then there is no point in having the check
| iris.save(cubes, | ||
| save_name | ||
| ) |
There was a problem hiding this comment.
| iris.save(cubes, | |
| save_name | |
| ) | |
| iris.save(cubes, save_name) |
| logger.info(f'Loading {in_dir}/{file}{year}{month:02d}.nc') | ||
| cube = iris.load_cube(f'{in_dir}/{file}{year}{month:02d}*.nc', | ||
| variable_list | ||
| ) |
There was a problem hiding this comment.
your code formatting is not following standard rules, please run a yapf -i on your codes when you done editing
This pull request is part of the CMUG work package on uncertainty propagation in ESMValTool with @morobking and @axel-lauer. It is part of issue #3408.
A separate pull request will be made in ESMValCore on branch CCI_LST_V3_CMOR_tables
Description
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.
New or updated recipe/diagnostic
New or updated data reformatting script
To help with the number of pull requests: