Skip to content

Commit

Permalink
Clean up docstrings (NOAA-GFDL#673)
Browse files Browse the repository at this point in the history
* clean up bad syntax and typos in docstrings
clean up comments
fix formatting

* clean up util module docstrings and comments

* remove unused parm from signal_logger and calls
  • Loading branch information
wrongkindofdoctor authored Aug 23, 2024
1 parent 0a00b60 commit d446b8d
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 41 deletions.
6 changes: 3 additions & 3 deletions src/cmip6.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
"""Code to parse CMIP6 controlled vocabularies and elements of the CMIP6 DRS.
Specifications for the above were taken from the CMIP6 `planning document
<http://goo.gl/v1drZl>`__. This was accessed at `<http://goo.gl/v1drZl>`__ -- we
<https://goo.gl/v1drZl>`__. This was accessed at `<https://goo.gl/v1drZl>`__ -- we
aren't aware of a permanent URL for this information.
The CMIP6 controlled vocabularies (lists of registered MIPs, modeling centers, etc.)
are derived from data in the
`PCMDI/cmip6-cmor-tables <https://github.com/PCMDI/cmip6-cmor-tables>`__
repo, which is included as a git subtree under ``/data``.
.. warning::
. warning::
Functionality here has been added as needed for the project and is incomplete.
For example, parsing subexperiments is not supported.
For example, parsing sub-experiments is not supported.
"""
import os
import re
Expand Down
2 changes: 1 addition & 1 deletion src/data_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def value(self):
@abc.abstractmethod
def is_scalar(self):
"""Whether the coordinate is a `scalar coordinate
<http://cfconventions.org/Data/cf-conventions/cf-conventions-1.8/cf-conventions.html#scalar-coordinate-variables>`__
<https://cfconventions.org/Data/cf-conventions/cf-conventions-1.8/cf-conventions.html#scalar-coordinate-variables>`__
(bool).
"""
pass
Expand Down
4 changes: 2 additions & 2 deletions src/environment_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -597,13 +597,13 @@ def tear_down(self):
self.env_mgr.destroy_environment(env)
self.env_mgr.tear_down()

def runtime_terminate(self, signum=None, frame=None):
def runtime_terminate(self, signum=None):
"""Handler called in the event that POD execution was halted abnormally,
by receiving ``SIGINT`` or ``SIGTERM``.
"""
# try to clean up everything
for p in self.pods:
util.signal_logger(self.__class__.__name__, signum, frame, log=p.pod.log)
util.signal_logger(self.__class__.__name__, signum, log=p.pod.log)
p.tear_down()
p.pod.close_log_file(log=True)

Expand Down
33 changes: 18 additions & 15 deletions src/preprocessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ class PrecipRateToFluxFunction(PreprocessorFunctionBase):
_flux_d = {tup[1]: tup[0] for tup in _std_name_tuples}

def edit_request(self, v: varlist_util.VarlistEntry, **kwargs):
"""Edit *pod*\'s Varlist prior to query. If the
"""Edit *pod*'s Varlist prior to query. If the
:class:`~src.diagnostic.VarlistEntry` *v* has a ``standard_name`` in the
recognized list, insert an alternate VarlistEntry whose translation
requests the complementary type of variable (i.e., if given rate, add an
Expand Down Expand Up @@ -290,7 +290,7 @@ class RenameVariablesFunction(PreprocessorFunctionBase):

def execute(self, var, ds, **kwargs):
"""Change the names of the DataArrays with Dataset *ds* to the names
specified by the :class:`~src.diagnostic.VarlistEntry` *var*. Names of
specified by the :class:`~src.varlist_util.VarlistEntry` *var*. Names of
the dependent variable and all dimension coordinates and scalar
coordinates (vertical levels) are changed in-place.
"""
Expand Down Expand Up @@ -401,7 +401,7 @@ def execute(self, var, ds, **kwargs):
class ExtractLevelFunction(PreprocessorFunctionBase):
"""Extract a requested pressure level from a Dataset containing a 3D variable.
.. note::
. note::
Unit conversion on the vertical coordinate is implemented, but
parametric vertical coordinates and coordinate interpolation are not.
Expand Down Expand Up @@ -536,19 +536,19 @@ class ApplyScaleAndOffsetFunction(PreprocessorFunctionBase):
"""If the Dataset has ``scale_factor`` and ``add_offset`` attributes set,
apply the corresponding constant linear transformation to the dependent
variable's values and unset these attributes. See `CF convention documentation
<http://cfconventions.org/Data/cf-conventions/cf-conventions-1.8/cf-conventions.html#attribute-appendix>`__
<https://cfconventions.org/Data/cf-conventions/cf-conventions-1.8/cf-conventions.html#attribute-appendix>`__
on the ``scale_factor`` and ``add_offset`` attributes.
.. note::
. note::
By default this function is not applied. It's only provided to implement
By default, this function is not applied. It's only provided to implement
workarounds for running the package on data with metadata (i.e., units)
that are known to be incorrect.
"""

def edit_request(self, v: varlist_util.VarlistEntry, **kwargs):
"""Edit the *pod*'s :class:`~src.VarlistEntry.Varlist` prior to data query.
If given a :class:`~src.VarlistEntry` *v* has a
"""Edit the *pod*'s :class:`~src.varlist_util.VarlistEntry.Varlist` prior to data query.
If given a :class:`~src.varlist_util.VarlistEntry` *v* has a
``scalar_coordinate`` for the Z axis (i.e., is requesting data on a
pressure level), return a copy of *v* with that ``scalar_coordinate``
removed (i.e., requesting a full 3D variable) to be used as an alternate
Expand Down Expand Up @@ -788,8 +788,8 @@ def check_time_bounds(self, ds, var: translation.TranslatedVarlistEntry, freq: s
raise IndexError(err_str)

def normalize_group_time_vals(self, time_vals: np.ndarray) -> np.ndarray:
"""Apply logic to fomat time_vals lists found in
check_grouo_daterange and convert them into str type.
"""Apply logic to format time_vals lists found in
check_group_daterange and convert them into str type.
This function also handles missing leading zeros
"""
poss_digits = [6,8,10,12,14]
Expand Down Expand Up @@ -823,18 +823,21 @@ def check_group_daterange(self, group_df: pd.DataFrame, case_dr,
group_df['start_time'] = pd.Series(start_times)
group_df['end_time'] = pd.Series(end_times)
else:
raise AttributeError('Data catalog is missing attributes `start_time` and/or `end_time` and can not infer from `time_range`')
raise AttributeError('Data catalog is missing attributes `start_time` and/or'
' `end_time` and can not infer from `time_range`')
try:
start_time_vals = self.normalize_group_time_vals(group_df['start_time'].values.astype(str))
end_time_vals = self.normalize_group_time_vals(group_df['end_time'].values.astype(str))
if not isinstance(start_time_vals[0], datetime.date):
date_format = dl.date_fmt(start_time_vals[0])
# convert start_times to date_format for all files in query
group_df['start_time'] = start_time_vals
group_df['start_time'] = group_df['start_time'].apply(lambda x: datetime.datetime.strptime(x, date_format))
group_df['start_time'] = group_df['start_time'].apply(lambda x:
datetime.datetime.strptime(x, date_format))
# convert end_times to date_format for all files in query
group_df['end_time'] = end_time_vals
group_df['end_time'] = group_df['end_time'].apply(lambda x: datetime.datetime.strptime(x, date_format))
group_df['end_time'] = group_df['end_time'].apply(lambda x:
datetime.datetime.strptime(x, date_format))
# method throws ValueError if ranges aren't contiguous
dates_df = group_df.loc[:, ['start_time', 'end_time']]
date_range_vals = []
Expand Down Expand Up @@ -1012,7 +1015,8 @@ def query_catalog(self,
try:
self.check_time_bounds(cat_dict[case_name], var.translation, freq)
except LookupError:
var.log.error(f'Data not found in catalog query for {var.translation.name} for requested date_range.')
var.log.error(f'Data not found in catalog query for {var.translation.name}'
f' for requested date_range.')
raise SystemExit("Terminating program")
return cat_dict

Expand Down Expand Up @@ -1451,7 +1455,6 @@ def rename_dataset_vars(self, ds: dict, case_list: dict) -> dict:
return ds



class DaskMultiFilePreprocessor(MDTFPreprocessorBase):
"""A Preprocessor class that uses xarray's dask support to
preprocess model data provided as one or multiple netcdf files per
Expand Down
3 changes: 1 addition & 2 deletions src/translation.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@
@util.mdtf_dataclass
class TranslatedVarlistEntry(data_model.DMVariable):
"""Class returned by :meth:`VarlistTranslator.translate`. Marks some
attributes inherited from :class:`~data_model.DMVariable` as being queryable
in :meth:`~data_manager.DataframeQueryDataSourceBase.query_dataset`.
attributes inherited from :class:`~data_model.DMVariable`.
"""
# to be more correct, we should probably have VarlistTranslator return a
# DMVariable, which is converted to this type on assignment to the
Expand Down
4 changes: 2 additions & 2 deletions src/util/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -589,14 +589,14 @@ def splice_into_list(list_, splice_d, key_fn=None, log=_log):
"""Splice sub-lists (values of *splice_d*) into list *list_* after their
corresponding entries (keys of *slice_d*). Example:
.. code-block:: python
. code-block:: python
>>> splice_into_list(['a','b','c'], {'b': ['b1', 'b2']})
['a', 'b', 'b1', 'b2', 'c']
Args:
list_ (list): Parent list to splice sub-lists into.
splice_d (dict): Sub-lists to splice in. Keys are entries in *list\_*
splice_d (dict): Sub-lists to splice in. Keys are entries in *list_*
and values are the sub-lists to insert after that entry. Duplicate
or missing entries are handled appropriately.
key_fn (function): Optional. If supplied, function applied to elements
Expand Down
6 changes: 3 additions & 3 deletions src/util/dataclass.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,12 @@ def update_defaults(self, d):
self._update_fields()

def match(self, str_, *args):
"""Match *str\_* using Python :py:func:`re.fullmatch` with *regex* and
"""Match *str_* using Python :py:func:`re.fullmatch` with *regex* and
populate object's fields according to the values captured by the named
capture groups in *regex*.
Args:
str\_ (str): Input string to parse.
str_ (str): Input string to parse.
args: Optional. Flags (as defined in Python :py:mod:`re`) to use in
the :py:func:`re.fullmatch` method of the *regex* and *match_error_filter*
(if defined.)
Expand Down Expand Up @@ -772,7 +772,7 @@ def _new_init(self, first_arg=None, *args, **kwargs):
type.__setattr__(cls, '__init__', _new_init)

def _from_string(cls_, str_, *args):
"""Create an object instance from a string representation *str\_*.
"""Create an object instance from a string representation *str_*.
Used by :func:`regex_dataclass` for parsing field values and automatic
type coercion.
"""
Expand Down
7 changes: 2 additions & 5 deletions src/util/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,16 +347,13 @@ def append_html_template(template_file: str, target_file: str, template_dict: di
html_str = _DoubleBraceTemplate(html_str).safe_substitute(template_dict)
if not os.path.exists(target_file):
if create:
# print("\tDEBUG: write {} to new {}".format(template_file, target_file))
mode = 'w'
else:
raise OSError("Can't find {}".format(target_file))
else:
if append:
# print("\tDEBUG: append {} to {}".format(template_file, target_file))
mode = 'a'
else:
# print("\tDEBUG: overwrite {} with {}".format(target_file, template_file))
os.remove(target_file)
mode = 'w'
with io.open(target_file, mode, encoding='utf-8') as f:
Expand Down Expand Up @@ -417,7 +414,7 @@ def cleanup(self):
for d in self._dirs:
self.rm_tempdir(d)

def tempdir_cleanup_handler(self, frame=None, signum=None):
def tempdir_cleanup_handler(self, signum=None):
# delete temp files
signal_logger(self.__class__.__name__, signum, frame, log=_log)
signal_logger(self.__class__.__name__, signum, log=_log)
self.cleanup()
3 changes: 1 addition & 2 deletions src/util/logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -654,14 +654,13 @@ def mdtf_log_header(title):
return str_ + (80 * '-') + '\n\n'


def signal_logger(caller_name: str, signum=None, frame=None, log=_log):
def signal_logger(caller_name: str, signum=None, log=_log):
"""Lookup signal name from number and write to log. Taken from
`<https://stackoverflow.com/a/2549950>`__.
Args:
caller_name (str): Calling function name, only used in log message.
signum: Signal number of the signal we received.
frame: parameters of received signal
log: log file
"""
if signum:
Expand Down
12 changes: 6 additions & 6 deletions src/verify_links.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env python
"""
Checks html links in the output of the files returned by a run of the MDTF
Checks html links in the output of the files returned by a run of the MDTF-diagnostics package
package and verifies that all linked files exist.
This is called by default at the end of each run, to determine if any PODs have
Expand All @@ -11,8 +11,8 @@
import sys

# do version check before importing other stuff
if sys.version_info[0] != 3 or sys.version_info[1] < 10:
sys.exit("ERROR: MDTF currently only supports python >= 3.10.*. Please check "
if sys.version_info[0] != 3 or sys.version_info[1] < 11:
sys.exit("ERROR: MDTF-diagnostics currently only supports python >= 3.12.*. Please check "
"which version is on your $PATH (e.g. with `which python`.)\n"
f"Attempted to run with following python version:\n{sys.version}")
# passed; continue with imports
Expand Down Expand Up @@ -91,7 +91,7 @@ def munge_input_url(url):
if not path_.endswith('/'):
path_ = path_ + '/'
url_parts = url_parts._replace(path=path_)
return (urllib.parse.urlunsplit(url_parts), path_, file_)
return urllib.parse.urlunsplit(url_parts), path_, file_

self.verbose = verbose
self.pod_name = None
Expand Down Expand Up @@ -153,7 +153,7 @@ def check_one_url(self, link):
except urllib.error.URLError as e:
# print('\nFailed to find file or connect to server.')
# print('Reason: ', e.reason)
tup = re.split(r"\[Errno 2\] No such file or directory: \'(.*)\'",
tup = re.split(r"\[Errno 2] No such file or directory: \'(.*)\'",
str(e.reason))
if len(tup) == 3:
str_ = util.abbreviate_path(tup[1], self.WORK_DIR, '$WORK_DIR')
Expand Down Expand Up @@ -188,7 +188,7 @@ def breadth_first(self, root_url):
link.target couldn't be found.
"""
missing = []
known_urls = set([root_url])
known_urls = {root_url}
root_parts = urllib.parse.urlsplit(root_url)
root_parts = root_parts._replace(path=os.path.dirname(root_parts.path))
# root_parent = URL to directory containing file referred to in root_url
Expand Down

0 comments on commit d446b8d

Please sign in to comment.