Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 47 additions & 20 deletions .github/scripts/pr_mod_file_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
Github Pull Request (PR), using the PyGithub interface,
and then to run tests on those files when appropriate.

Note: This version currently limit the tests to a subset of files,
in order to avoid running pylint on non-core python source files.
Note: This version currently limits the tests to the python files
under the "lib" directory, in order to avoid running pylint on
non-core python source files.

Written by: Jesse Nusbaumer <nusbaume@ucar.edu> - November, 2020
"""
Expand All @@ -24,7 +25,9 @@
import argparse

from stat import S_ISREG
from github import Github
from pathlib import Path

from github import Auth, Github

#Local scripts:
from pylint_threshold_test import pylint_check
Expand Down Expand Up @@ -93,6 +96,32 @@ def _file_is_python(filename):
#Return file type result:
return is_python

#################

def _file_is_testable(filename, testable_dir, excluded_dirs):

"""
Checks whether a given file lives underneath a
directory whose python files should be linted, while
also skipping any files that live underneath one of
the excluded directories.
"""

#Determine all directories that contain this file:
file_parents = Path(filename).parents

#File must live somewhere underneath the testable directory:
if Path(testable_dir) not in file_parents:
return False

#File must not live underneath an excluded directory:
for excluded_dir in excluded_dirs:
if Path(excluded_dir) in file_parents:
return False

#If both checks pass, then the file is testable:
return True

#++++++++++++++++++++++++++++++
#Input Argument parser function
#++++++++++++++++++++++++++++++
Expand Down Expand Up @@ -140,18 +169,15 @@ def _main_prog():

print("Generating list of modified files...")

# This should eventually be passed in via a command-line
# argument, and include everything inside the "lib" directory -JN:
testable_files = {
"lib/adf_base.py",
"lib/adf_config.py",
"lib/adf_file_utils.py",
"lib/adf_info.py",
"lib/adf_obs.py",
"lib/adf_units.py",
"lib/adf_web.py",
"lib/adf_diag.py",
}
#All python files underneath this directory are linted. This
#should eventually be passed in via a command-line argument -JN:
testable_dir = "lib"

#Directories underneath "testable_dir" that should never be linted.
#The "lib/externals" directory contains code copied in from other
#projects (e.g. CVDP), which needs to stay identical to its upstream
#source.
excluded_dirs = {"lib/externals"}

#+++++++++++++++++++++++
#Read in input arguments
Expand All @@ -169,7 +195,7 @@ def _main_prog():
#Log-in to github API using token
#++++++++++++++++++++++++++++++++

ghub = Github(token)
ghub = Github(auth=Auth.Token(token))

#++++++++++++++++++++
#Open ESCOMP/CAM repo
Expand All @@ -189,7 +215,7 @@ def _main_prog():
#++++++++++++++++++++++++++++++

#Create empty list to store python files:
pyfiles = list()
pyfiles = []

#Extract Github file objects:
file_obj_list = pull_req.get_files()
Expand All @@ -215,7 +241,7 @@ def _main_prog():
# users of python files that will be tested:
lint_files = []
for pyfile in pyfiles:
if pyfile in testable_files:
if _file_is_testable(pyfile, testable_dir, excluded_dirs):
lint_files.append(pyfile)
else:
continue
Expand Down Expand Up @@ -261,9 +287,10 @@ def _main_prog():
print("All pylint tests passed!")
sys.exit(0)

#If no python files in set of testable_files, then exit script:
#If no python files are underneath "testable_dir", then exit script:
else:
print("No ADF classes were modified in PR, so there is nothing to test.")
print(f"No python files under '{testable_dir}' were modified in PR, "
"so there is nothing to test.")
sys.exit(0)

#End if (lint_files)
Expand Down
17 changes: 12 additions & 5 deletions .github/workflows/ADF_linting.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,18 @@ jobs:
# install required python packages
- name: Install dependencies
run: |
python -m pip install --upgrade pip # Install latest version of PIP
pip install PyGithub # Install PyGithub python package
pip install pylint # Install Pylint python package
pip install pyyaml # Install PyYAML python package
pip install numpy # Install NumPy python package
python -m pip install --upgrade pip
pip install PyGithub
pip install pylint
pip install pyyaml
pip install numpy
pip install xarray
pip install pandas
pip install matplotlib
pip install cartopy
pip install geocat-comp
pip install markdown
pip install jinja2
# run CAM source code testing master script:
- name: source-code testing python script
env:
Expand Down
2 changes: 1 addition & 1 deletion lib/adf_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class AdfBase:

def __init__(self, debug=False):
"""
Initalize CAM diagnostics object.
Initialize CAM diagnostics object.
"""

# Check that debug is in fact a boolean,
Expand Down
2 changes: 1 addition & 1 deletion lib/adf_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ def __expand_yaml_var_ref(self, var_val):
# --------------------------

# Throw an error if keyword not in dictionary:
if kword_match_str_key not in self.__search_dict.keys():
if kword_match_str_key not in self.__search_dict:
ermsg = f"ERROR: Variable '{kword_match_str}'"
ermsg += " not found in config (YAML) file."
self.end_diag_fail(ermsg)
Expand Down
2 changes: 1 addition & 1 deletion lib/adf_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
# apply scaling.


class AdfData:
class AdfData: # pylint: disable=too-many-public-methods
"""A class instantiated with an AdfDiag object.
Methods provide means to load data.
This class does not interact with plotting,
Expand Down
2 changes: 1 addition & 1 deletion lib/adf_derive.py
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ def derive_variable(
ds = self.data.load_dataset(constit_files)
if not ds:
dmsg = f"derived time series for {case_name}:"
dmsg += f"\n\tNo files to open."
dmsg += "\n\tNo files to open."
self.debug_log(dmsg)
return

Expand Down
3 changes: 2 additions & 1 deletion lib/adf_diag.py
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,8 @@ def run_pool(commands, label):
for hist_str in hist_str_case:

print(
f"\t Processing time series for {case_type_string} {case_name}, {hist_str} files:"
f"\t Processing time series for {case_type_string} {case_name},"
f" {hist_str} files:"
)
if not list(starting_location.glob("*" + hist_str + ".*.nc")):
emsg = f"No history *{hist_str}.*.nc files found in '{starting_location}'."
Expand Down
2 changes: 1 addition & 1 deletion lib/adf_file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ def find_ts_files(ts_loc, pattern, recursive=True):
return sorted(ts_loc.rglob(pattern))


def select_ts_files(fils, syr, eyr):
def select_ts_files(fils, syr, eyr): # pylint: disable=too-many-return-statements
"""
Narrow a set of time series files to those needed for a year range.

Expand Down
9 changes: 7 additions & 2 deletions lib/adf_gents.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
import sys
from pathlib import Path

# +++++++++++++++++++++++++++++++++++++++++++++++++
# import non-standard python modules, including ADF
# +++++++++++++++++++++++++++++++++++++++++++++++++

import xarray as xr

# ADF modules:
Expand Down Expand Up @@ -177,7 +181,8 @@ def create_time_series_gents(adf, baseline=False):
from :mod:`adf_file_utils`.
"""

HFCollection, TSCollection = _import_gents()
# These are classes, so keep their PascalCase names:
HFCollection, TSCollection = _import_gents() # pylint: disable=invalid-name

# Notify user that script has started:
msg = "\n Calculating CAM time series with GenTS..."
Expand Down Expand Up @@ -344,7 +349,7 @@ def create_time_series_gents(adf, baseline=False):
tsc = _restrict_to_vars(tsc, wanted_vars)
# End if

if not len(tsc):
if not tsc:
wmsg = (
f"\t WARNING: GenTS found nothing to generate for '{hist_str}'."
)
Expand Down
1 change: 0 additions & 1 deletion lib/adf_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import copy
import os
import getpass
import subprocess

# +++++++++++++++++++++++++++++++++++++++++++++++++
# import non-standard python modules, including ADF
Expand Down
2 changes: 1 addition & 1 deletion lib/adf_obs.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Diagnostics Framework (ADF).
This class inherits from the AdfInfo class.

Currently this class does three things:
Currently this class does four things:

1. Initializes an instance of AdfInfo.

Expand Down
2 changes: 1 addition & 1 deletion lib/adf_units.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
asked, and getting it wrong scales the data twice.

Rendering a unit for somewhere with no LaTeX renderer, such as a table cell,
is `adf_utils.plain_text_units`; this module only compares.
is :func:`adf_utils.plain_text_units`; this module only compares.
"""

import re
Expand Down
Loading
Loading