From ac5b30cd7159e34821002820fcbedbe8c0d928f7 Mon Sep 17 00:00:00 2001 From: Ian Laflotte Date: Mon, 12 Aug 2024 12:21:15 -0400 Subject: [PATCH] more pylint cleanup! more snake case enforcement. variables appearing unused to pylint b.c. of click conventions ignored in more places some unnecessary returns/passes/if-else conditions removed/rewritten a doc string here and there --- fre/app/__init__.py | 4 +- fre/app/freapp.py | 4 +- fre/app/maskAtmosPlevel.py | 154 -------------------------- fre/catalog/frecatalog.py | 9 +- fre/check/frecheck.py | 4 + fre/check/frecheckexample.py | 2 +- fre/cmor/frecmor.py | 4 + fre/list/frelist.py | 6 +- fre/list/frelistexample.py | 2 +- fre/pp/frepp.py | 204 +++++++++++++---------------------- fre/run/frerun.py | 1 + fre/run/frerunexample.py | 4 +- fre/test/fretest.py | 5 + fre/test/fretestexample.py | 4 +- 14 files changed, 113 insertions(+), 294 deletions(-) delete mode 100755 fre/app/maskAtmosPlevel.py diff --git a/fre/app/__init__.py b/fre/app/__init__.py index 04108375..3bc69f53 100644 --- a/fre/app/__init__.py +++ b/fre/app/__init__.py @@ -1,5 +1,5 @@ ''' for fre.app imports ''' -from .maskAtmosPlevel import maskAtmosPlevel_subtool +from .mask_atmos_plevel import mask_atmos_plevel_subtool from .freapp import app_cli -__all__ = ["maskAtmosPlevel_subtool", "app_cli"] +__all__ = ["mask_atmos_plevel_subtool", "app_cli"] diff --git a/fre/app/freapp.py b/fre/app/freapp.py index 133b2193..cdb0ff9a 100644 --- a/fre/app/freapp.py +++ b/fre/app/freapp.py @@ -5,7 +5,7 @@ import click -from .maskAtmosPlevel import maskAtmosPlevel_subtool +from .mask_atmos_plevel import mask_atmos_plevel_subtool from .generate_time_averages.generate_time_averages import generate @click.group(help=click.style(" - access fre app subcommands", fg=(250,154,90))) @@ -29,7 +29,7 @@ def app_cli(): def mask_atmos_plevel(context, infile, outfile, psfile): # pylint: disable=unused-argument """Mask out pressure level diagnostic output below land surface""" - context.forward(maskAtmosPlevel_subtool) + context.forward(mask_atmos_plevel_subtool) @app_cli.command() diff --git a/fre/app/maskAtmosPlevel.py b/fre/app/maskAtmosPlevel.py deleted file mode 100755 index fd6d5896..00000000 --- a/fre/app/maskAtmosPlevel.py +++ /dev/null @@ -1,154 +0,0 @@ -#!/usr/bin/env python - -# This script contains the refineDiags that produce data at the same -# frequency as the input data (no reduction) such as surface albedo, -# masking fields,... -# It can accept any file and will only compute refineDiags in fields -# are present. - -import os -import netCDF4 as nc -import xarray as xr -import click - -@click.command() -def maskAtmosPlevel_subtool(infile, outfile, psfile): - # Error if outfile exists - if os.path.exists(outfile): - raise Exception(f"ERROR: Output file '{outfile}' already exists") - - # Open ps dataset - if not os.path.exists(psfile): - raise Exception(f"ERROR: Input surface pressure file '{psfile}' does not exist") - ds_ps = xr.open_dataset(psfile) - - # Exit with message if "ps" not available - if "ps" not in list(ds_ps.variables): - print(f"WARNING: File '{infile}' does not contain surface pressure, so exiting") - return None - - # Open input dataset - if not os.path.exists(infile): - raise Exception(f"ERROR: Input file '{infile}' does not exist") - ds_in = xr.open_dataset(infile) - - # The trigger for atmos masking is a variable attribute "needs_atmos_masking = True". - # In the future this will be set within the model, but for now and testing, - # we'll add the attribute for variables that end with "_unmsk". - ds_in = preprocess(ds_in) - - ds_out = xr.Dataset() - - # Process all variables with attribute "needs_atmos_masking = True" - for var in list(ds_in.variables): - if 'needs_atmos_masking' in ds_in[var].attrs: - del ds_in[var].attrs['needs_atmos_masking'] - ds_out[var] = mask_field_above_surface_pressure(ds_in, var, ds_ps) - else: - continue - - # Write the output file if anything was done - if ds_out.variables: - print(f"Modifying variables '{list(ds_out.variables)}', appending into new file '{outfile}'") - write_dataset(ds_out, ds_in, outfile) - else: - print(f"No variables modified, so not writing output file '{outfile}'") - return None - - -def preprocess(ds): - """add needs_atmos_masking attribute if var ends with _unmsk""" - - for var in list(ds.variables): - if var.endswith('_unmsk'): - ds[var].attrs['needs_atmos_masking'] = True - - return ds - - -def mask_field_above_surface_pressure(ds, var, ds_ps): - """mask data with pressure larger than surface pressure""" - - plev = pressure_coordinate(ds, var) - - # broadcast pressure coordinate and surface pressure to - # the dimensions of the variable to mask - plev_extended, _ = xr.broadcast(plev, ds[var]) - ps_extended, _ = xr.broadcast(ds_ps["ps"], ds[var]) - # masking do not need looping - masked = xr.where(plev_extended > ps_extended, 1.0e20, ds[var]) - # copy attributes, but it doesn't include the missing values - attrs = ds[var].attrs.copy() - # add the missing values back - attrs['missing_value'] = 1.0e20 - attrs['_FillValue'] = 1.0e20 - masked.attrs = attrs - # transpose dims like the original array - masked = masked.transpose(*ds[var].dims) - - print(f"Processed {var}") - - return masked - - -def pressure_coordinate(ds, varname, verbose=False): - """check if dataArray has pressure coordinate fitting requirements - and return it""" - - pressure_coord = None - - for dim in list(ds[varname].dims): - if dim in list(ds.variables): # dim needs to have values in file - if ds[dim].attrs["long_name"] == "pressure": - pressure_coord = ds[dim] - elif ("coordinates" in ds.attrs) and (ds[dim].attrs["units"] == "Pa"): - pressure_coord = ds[dim] - - return pressure_coord - - -def write_dataset(ds, template, outfile): - """prepare the dataset and dump into netcdf file""" - - # copy global attributes - ds.attrs = template.attrs.copy() - - # copy all variables and their attributes - # except those already processed - for var in list(template.variables): - if var in list(ds.variables): - continue - else: - ds[var] = template[var] - ds[var].attrs = template[var].attrs.copy() - - ds.to_netcdf(outfile, unlimited_dims="time") - - return None - - -def set_netcdf_encoding(ds, pressure_vars): - """set preferred options for netcdf encoding""" - - all_vars = list(ds.variables) - encoding = {} - - for var in do_not_encode_vars + pressure_vars: - if var in all_vars: - encoding.update({var: dict(_FillValue=None)}) - - return encoding - - -def post_write(filename, ds, var_with_bounds, bounds_variables): - """fix a posteriori attributes that xarray.to_netcdf - did not do properly using low level netcdf lib""" - - f = nc.Dataset(filename, "a") - - for var, bndvar in zip(var_with_bounds, bounds_variables): - f.variables[var].setncattr("bounds", bndvar) - - f.close() - - return None diff --git a/fre/catalog/frecatalog.py b/fre/catalog/frecatalog.py index 008d187e..3223002e 100644 --- a/fre/catalog/frecatalog.py +++ b/fre/catalog/frecatalog.py @@ -1,5 +1,9 @@ +''' +entry point for fre catalog subcommands +''' + import click -import catalogbuilder +#import catalogbuilder from catalogbuilder.scripts import gen_intake_gfdl from catalogbuilder.scripts import test_catalog @@ -33,7 +37,8 @@ def builder(context, input_path = None, output_path = None, config = None, filte @catalog_cli.command() @click.argument('json_path', nargs = 1 , required = True) @click.argument('json_template_path', nargs = 1 , required = False) -@click.option('-tf', '--test-failure', is_flag=True, default = False, help="Errors are only printed. Program will not exit.") +@click.option('-tf', '--test-failure', is_flag=True, default = False, + help="Errors are only printed. Program will not exit.") @click.pass_context def validate(context, json_path, json_template_path, test_failure): # pylint: disable=unused-argument diff --git a/fre/check/frecheck.py b/fre/check/frecheck.py index 685720f8..9640d138 100644 --- a/fre/check/frecheck.py +++ b/fre/check/frecheck.py @@ -1,4 +1,7 @@ +''' fre check ''' + import click + from .frecheckexample import check_test_function @click.group(help=click.style(" - access fre check subcommands", fg=(162,91,232))) @@ -9,6 +12,7 @@ def check_cli(): @click.option('--uppercase', '-u', is_flag=True, help = 'Print statement in uppercase.') @click.pass_context def function(context, uppercase): + # pylint: disable=unused-argument """ - Execute fre check test """ context.forward(check_test_function) diff --git a/fre/check/frecheckexample.py b/fre/check/frecheckexample.py index aa9e84b6..daac462a 100644 --- a/fre/check/frecheckexample.py +++ b/fre/check/frecheckexample.py @@ -7,7 +7,7 @@ import click @click.command() -def check_test_function(uppercase): +def check_test_function(uppercase=None): """Execute fre list testfunction2.""" statement = "testingtestingtestingtesting" if uppercase: diff --git a/fre/cmor/frecmor.py b/fre/cmor/frecmor.py index f1bfe1cc..c693b1e6 100644 --- a/fre/cmor/frecmor.py +++ b/fre/cmor/frecmor.py @@ -1,4 +1,7 @@ +''' fre cmor ''' + import click + from .CMORmixer import cmor_run_subtool @click.group(help=click.style(" - access fre cmor subcommands", fg=(232,91,204))) @@ -28,6 +31,7 @@ def cmor_cli(): required=True) @click.pass_context def run(context, indir, outdir, varlist, table_config, exp_config): + # pylint: disable=unused-argument """Rewrite climate model output""" context.forward(cmor_run_subtool) diff --git a/fre/list/frelist.py b/fre/list/frelist.py index e5874b6b..fea3b6ea 100644 --- a/fre/list/frelist.py +++ b/fre/list/frelist.py @@ -1,14 +1,18 @@ +''' fre list ''' + import click + from .frelistexample import list_test_function @click.group(help=click.style(" - access fre list subcommands", fg=(232,204,91))) def list_cli(): - pass + ''' entry point to fre list click commands ''' @list_cli.command() @click.option('--uppercase', '-u', is_flag=True, help = 'Print statement in uppercase.') @click.pass_context def function(context, uppercase): + # pylint: disable=unused-argument """ - Execute fre list test """ context.forward(list_test_function) diff --git a/fre/list/frelistexample.py b/fre/list/frelistexample.py index 1effeb9e..7e40e195 100644 --- a/fre/list/frelistexample.py +++ b/fre/list/frelistexample.py @@ -7,7 +7,7 @@ import click @click.command() -def list_test_function(uppercase): +def list_test_function(uppercase=None): """Execute fre list testfunction2.""" statement = "testingtestingtestingtesting" if uppercase: diff --git a/fre/pp/frepp.py b/fre/pp/frepp.py index 7ca8bf61..b3456e31 100644 --- a/fre/pp/frepp.py +++ b/fre/pp/frepp.py @@ -1,4 +1,3 @@ -#!/usr/bin/env python3 ''' fre pp ''' import click @@ -13,167 +12,129 @@ @click.group(help=click.style(" - access fre pp subcommands", fg=(57,139,210))) def pp_cli(): - pass + ''' entry point to fre pp click commands ''' + # fre pp status @pp_cli.command() -@click.option("-e", - "--experiment", - type=str, +@click.option("-e", "--experiment", type=str, help="Experiment name", required=True) -@click.option("-p", - "--platform", - type=str, +@click.option("-p", "--platform", type=str, help="Platform name", required=True) -@click.option("-t", - "--target", - type=str, - help="Target name", - required=True) +@click.option("-t", "--target", type=str, + help="Target name", + required=True) @click.pass_context def status(context, experiment, platform, target): + # pylint: disable=unused-argument """ - Report status of PP configuration""" context.forward(status_subtool) # fre pp run @pp_cli.command() -@click.option("-e", - "--experiment", - type=str, +@click.option("-e", "--experiment", type=str, help="Experiment name", required=True) -@click.option("-p", - "--platform", - type=str, +@click.option("-p", "--platform", type=str, help="Platform name", required=True) -@click.option("-t", - "--target", - type=str, - help="Target name", - required=True) +@click.option("-t", "--target", type=str, + help="Target name", + required=True) @click.pass_context def run(context, experiment, platform, target): + # pylint: disable=unused-argument """ - Run PP configuration""" context.forward(pp_run_subtool) # fre pp validate @pp_cli.command() -@click.option("-e", - "--experiment", - type=str, +@click.option("-e", "--experiment", type=str, help="Experiment name", required=True) -@click.option("-p", - "--platform", - type=str, +@click.option("-p", "--platform", type=str, help="Platform name", required=True) -@click.option("-t", - "--target", - type=str, - help="Target name", - required=True) +@click.option("-t", "--target", type=str, + help="Target name", + required=True) @click.pass_context def validate(context, experiment, platform, target): + # pylint: disable=unused-argument """ - Validate PP configuration""" context.forward(validate_subtool) # fre pp install @pp_cli.command() -@click.option("-e", - "--experiment", - type=str, +@click.option("-e", "--experiment", type=str, help="Experiment name", required=True) -@click.option("-p", - "--platform", - type=str, +@click.option("-p", "--platform", type=str, help="Platform name", required=True) -@click.option("-t", - "--target", - type=str, - help="Target name", - required=True) +@click.option("-t", "--target", type=str, + help="Target name", + required=True) @click.pass_context def install(context, experiment, platform, target): + # pylint: disable=unused-argument """ - Install PP configuration""" context.forward(install_subtool) @pp_cli.command() -@click.option("-y", - "--yamlfile", - type=str, +@click.option("-y", "--yamlfile", type=str, help="YAML file to be used for parsing", required=True) -@click.option("-e", - "--experiment", - type=str, +@click.option("-e", "--experiment", type=str, help="Experiment name", required=True) -@click.option("-p", - "--platform", - type=str, +@click.option("-p", "--platform", type=str, help="Platform name", required=True) -@click.option("-t", - "--target", - type=str, - help="Target name", - required=True) +@click.option("-t", "--target", type=str, + help="Target name", + required=True) @click.pass_context def configure_yaml(context,yamlfile,experiment,platform,target): + # pylint: disable=unused-argument """ - Execute fre pp configure """ context.forward(yamlInfo) @pp_cli.command() -@click.option("-e", - "--experiment", - type=str, +@click.option("-e", "--experiment", type=str, help="Experiment name", required=True) -@click.option("-p", - "--platform", - type=str, +@click.option("-p", "--platform", type=str, help="Platform name", required=True) -@click.option("-t", - "--target", - type=str, - help="Target name", - required=True) -@click.option("-b", - "--branch", +@click.option("-t", "--target", type=str, + help="Target name", + required=True) +@click.option("-b", "--branch", show_default=True, - default="main", - type=str, - help=" ".join(["Name of fre2/workflows/postproc branch to clone;" - "defaults to 'main'. Not intended for production use," - "but needed for branch testing."]) - ) + default="main", type=str, + help="Name of fre2/workflows/postproc branch to clone; " \ + "defaults to 'main'. Not intended for production use, " \ + "but needed for branch testing." ) @click.pass_context def checkout(context, experiment, platform, target, branch='main'): + # pylint: disable=unused-argument """ - Execute fre pp checkout """ context.forward(checkoutTemplate) @pp_cli.command() -@click.option('-x', - '--xml', +@click.option('-x', '--xml', required=True, help="Required. The Bronx XML") -@click.option('-p', - '--platform', +@click.option('-p', '--platform', required=True, help="Required. The Bronx XML Platform") -@click.option('-t', - '--target', +@click.option('-t', '--target', required=True, help="Required. The Bronx XML Target") -@click.option('-e', - '--experiment', +@click.option('-e', '--experiment', required=True, help="Required. The Bronx XML Experiment") @click.option('--do_analysis', @@ -181,36 +142,34 @@ def checkout(context, experiment, platform, target, branch='main'): default=False, help="Optional. Runs the analysis scripts.") @click.option('--historydir', - help="Optional. History directory to reference. " \ - "If not specified, the XML's default will be used.") + help="Optional. History directory to reference. " \ + "If not specified, the XML's default will be used.") @click.option('--refinedir', - help="Optional. History refineDiag directory to reference. " \ - "If not specified, the XML's default will be used.") + help="Optional. History refineDiag directory to reference. " \ + "If not specified, the XML's default will be used.") @click.option('--ppdir', - help="Optional. Postprocessing directory to reference. " \ - "If not specified, the XML's default will be used.") + help="Optional. Postprocessing directory to reference. " \ + "If not specified, the XML's default will be used.") @click.option('--do_refinediag', is_flag=True, default=False, help="Optional. Process refineDiag scripts") @click.option('--pp_start', - help="Optional. Starting year of postprocessing. " \ - "If not specified, a default value of '0000' " \ - "will be set and must be changed in rose-suite.conf") + help="Optional. Starting year of postprocessing. " \ + "If not specified, a default value of '0000' " \ + "will be set and must be changed in rose-suite.conf") @click.option('--pp_stop', - help="Optional. Ending year of postprocessing. " \ - "If not specified, a default value of '0000' " \ + help="Optional. Ending year of postprocessing. " \ + "If not specified, a default value of '0000' " \ "will be set and must be changed in rose-suite.conf") @click.option('--validate', is_flag=True, - help="Optional. Run the Cylc validator " \ + help="Optional. Run the Cylc validator " \ "immediately after conversion") -@click.option('-v', - '--verbose', +@click.option('-v', '--verbose', is_flag=True, help="Optional. Display detailed output") -@click.option('-q', - '--quiet', +@click.option('-q', '--quiet', is_flag=True, help="Optional. Display only serious messages and/or errors") @click.option('--dual', @@ -219,42 +178,33 @@ def checkout(context, experiment, platform, target, branch='main'): @click.pass_context def configure_xml(context, xml, platform, target, experiment, do_analysis, historydir, refinedir, ppdir, do_refinediag, pp_start, pp_stop, validate, verbose, quiet, dual): + # pylint: disable=unused-argument """ - Converts a Bronx XML to a Canopy rose-suite.conf """ context.forward(convert) #fre pp wrapper @pp_cli.command() -@click.option("-e", - "--experiment", - type=str, +@click.option("-e", "--experiment", type=str, help="Experiment name", required=True) -@click.option("-p", - "--platform", - type=str, +@click.option("-p", "--platform", type=str, help="Platform name", required=True) -@click.option("-t", - "--target", - type=str, - help="Target name", - required=True) -@click.option("-c", - "--config-file", - type=str, - help="Path to a configuration file in either XML or YAML", - required=True) -@click.option("-b", - "--branch", +@click.option("-t", "--target", type=str, + help="Target name", + required=True) +@click.option("-c", "--config-file", type=str, + help="Path to a configuration file in either XML or YAML", + required=True) +@click.option("-b", "--branch", show_default=True, - default="main", - type=str, - help=" ".join(["Name of fre2/workflows/postproc branch to clone;" - "defaults to 'main'. Not intended for production use," - "but needed for branch testing."]) - ) + default="main", type=str, + help="Name of fre2/workflows/postproc branch to clone; " \ + "defaults to 'main'. Not intended for production use, " \ + "but needed for branch testing." ) @click.pass_context def wrapper(context, experiment, platform, target, config_file, branch='main'): + # pylint: disable=unused-argument """ - Execute fre pp steps in order """ context.forward(runFre2pp) diff --git a/fre/run/frerun.py b/fre/run/frerun.py index caa01315..c2c611c3 100644 --- a/fre/run/frerun.py +++ b/fre/run/frerun.py @@ -13,6 +13,7 @@ def run_cli(): @click.option('--uppercase', '-u', is_flag=True, help = 'Print statement in uppercase.') @click.pass_context def function(context, uppercase): + # pylint: disable=unused-argument """ - Execute fre run test """ context.forward(run_test_function) diff --git a/fre/run/frerunexample.py b/fre/run/frerunexample.py index 55cb1feb..a4335144 100644 --- a/fre/run/frerunexample.py +++ b/fre/run/frerunexample.py @@ -7,8 +7,8 @@ import click @click.command() -def run_test_function(uppercase): - """Execute fre list testfunction2.""" +def run_test_function(uppercase=None): + """Execute fre run run_test_function""" statement = "testingtestingtestingtesting" if uppercase: statement = statement.upper() diff --git a/fre/test/fretest.py b/fre/test/fretest.py index 04d6a1e8..172ac855 100644 --- a/fre/test/fretest.py +++ b/fre/test/fretest.py @@ -1,3 +1,7 @@ +''' +entry point for fre test subcommands +''' + import click from .fretestexample import test_test_function @@ -9,6 +13,7 @@ def test_cli(): @click.option('--uppercase', '-u', is_flag=True, help = 'Print statement in uppercase.') @click.pass_context def function(context, uppercase): + # pylint: disable=unused-argument """ - Execute fre test test """ context.forward(test_test_function) diff --git a/fre/test/fretestexample.py b/fre/test/fretestexample.py index ad465774..86d0f6ce 100644 --- a/fre/test/fretestexample.py +++ b/fre/test/fretestexample.py @@ -7,8 +7,8 @@ import click @click.command() -def test_test_function(uppercase): - """Execute fre list testfunction2.""" +def test_test_function(uppercase=None): + """Execute fre list test_test_function""" statement = "testingtestingtestingtesting" if uppercase: statement = statement.upper()