Merge pull request #425 from bknueven/ruff

Add `ruff` linter fixes and checks

Author: bknueven

Diff for: .github/workflows/test_pr_and_main.yml

@@ -17,9 +17,17 @@ defaults:
         shell: bash -l {0}
 
 jobs:
+  ruff:
+    name: Ruff Linting
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: chartboost/ruff-action@v1
+
   nompi4py:
     name: no mpi4py
     runs-on: ubuntu-latest
+    needs: [ruff]
     steps:
       - uses: actions/checkout@v3
       - uses: conda-incubator/setup-miniconda@v2
@@ -45,6 +53,7 @@ jobs:
   regression:
     name: Basic regression tests
     runs-on: ubuntu-latest
+    needs: [ruff]
 
     steps:
       - uses: actions/checkout@v3
@@ -86,6 +95,9 @@ jobs:
     name: run_all.py
     runs-on: ubuntu-latest
     timeout-minutes: 15
+    # this takes the most time, so we'll start it
+    # without waiting on the linting and other checks
+    # needs: [ruff]
 
     steps:
       - uses: actions/checkout@v3
@@ -117,6 +129,7 @@ jobs:
   schur-complement:
     name: schur-complement
     runs-on: ubuntu-latest
+    needs: [ruff]
     strategy:
       matrix:
         python-version: [3.8, 3.9]
@@ -151,6 +164,7 @@ jobs:
   straight-tests:
     name: straight_tests.py
     runs-on: ubuntu-latest
+    needs: [ruff]
 
     steps:
       - uses: actions/checkout@v3
@@ -176,6 +190,7 @@ jobs:
   admm-wrapper:
     name: admm wrapper tests
     runs-on: ubuntu-latest
+    needs: [ruff]
 
     steps:
       - uses: actions/checkout@v3
@@ -205,6 +220,7 @@ jobs:
   aph:
     name: aph tests
     runs-on: ubuntu-latest
+    needs: [ruff]
 
     steps:
       - uses: actions/checkout@v3
@@ -232,6 +248,7 @@ jobs:
   pickled-bundles:
     name: pickled bundles tests
     runs-on: ubuntu-latest
+    needs: [ruff]
 
     steps:
       - uses: actions/checkout@v3
@@ -259,6 +276,7 @@ jobs:
   confidence-intervals:
     name: confidence intervals tests
     runs-on: ubuntu-latest
+    needs: [ruff]
 
     steps:
       - uses: actions/checkout@v3
@@ -291,6 +309,7 @@ jobs:
   test-generic-pyomo-released:
     name: generic_tester.py
     runs-on: ubuntu-latest
+    needs: [ruff]
     timeout-minutes: 15
 
     steps:
@@ -323,6 +342,7 @@ jobs:
   test-gradient-rho:
     name: gradient and rho tests
     runs-on: ubuntu-latest
+    needs: [ruff]
 
     steps:
       - uses: actions/checkout@v3
@@ -354,7 +374,6 @@ jobs:
 
   test-headers:
     name: header test
-
     runs-on: ubuntu-latest
 
     steps:
@@ -381,8 +400,8 @@ jobs:
 
   test-pysp:
     name: pysp tests
-
     runs-on: ubuntu-latest
+    needs: [ruff]
 
     steps:
       - uses: actions/checkout@v3
@@ -415,8 +434,8 @@ jobs:
 
   test-cylinders:
     name: tests on some cylinders
-
     runs-on: ubuntu-latest
+    needs: [ruff]
 
     steps:
       - uses: actions/checkout@v3

Diff for: .ruff.toml

@@ -0,0 +1,7 @@
+# ignore old Pyomo models
+extend-exclude = [
+    "ReferenceModel*.py",
+    "./mpisppy/utils/callbacks/termination/tests/markshare2.py",
+    "./mpisppy/tests/examples/hydro/hydro.py",
+    "./examples/hydro/hydro.py",
+]

Diff for: examples/acopf3/ACtree.py

@@ -149,11 +149,9 @@ def __init__(self, Parent, TreeInfo, ScenarioList, Name, CondProb, acstream):
             self.kids = []
             if self.stage < TreeInfo.NumStages:
                 bf = TreeInfo.BFs[self.stage-1]
-                snstr = "_sn"+str(self.sn)
                 self.sn += 1 # serial number for non-leaf, non-ROOT nodes
             else:
                 bf = 1  # leaf node
-                snstr = ""
             for b in range(bf):
                 # divide up the scenario list
                 plist = self.ScenarioList # typing aid

Diff for: examples/acopf3/ccopf2wood.py

@@ -23,8 +23,7 @@
 # the problem
 import ACtree as etree
 from ccopf_multistage import pysp2_callback,\
-    scenario_denouement, _md_dict, FixFast, FixNever, FixGaussian
-import rho_setter
+    scenario_denouement, _md_dict, FixFast
 
 import pyomo.environ as pyo
 import socket

Diff for: examples/acopf3/ccopf_multistage.py

@@ -11,7 +11,6 @@
 import egret
 import egret.models.acopf as eac
 import egret.models.ac_relaxations as eac_relax
-from egret.data.model_data import ModelData
 from egret.parsers.matpower_parser import create_ModelData
 import mpisppy.scenario_tree as scenario_tree
 import mpisppy.utils.sputils as sputils
@@ -22,9 +21,7 @@
 import sys
 import copy
 import scipy
-import socket
 import numpy as np
-import datetime as dt
 import mpisppy.MPI as mpi
 
 import pyomo.environ as pyo
@@ -57,10 +54,10 @@ def FixGaussian(minutes, acstream, mu, sigma):
 #======= end repair functions =====
 
 def _md_dict(epath):
-    p = str(egret.__path__)
-    l = p.find("'")
-    r = p.find("'", l+1)
-    egretrootpath = p[l+1:r]
+    path = str(egret.__path__)
+    left = path.find("'")
+    right = path.find("'", left+1)
+    egretrootpath = path[left+1:right]
     if epath[0] != os.sep:
         test_case = os.path.join(egretrootpath, epath)
     else:
@@ -262,9 +259,10 @@ def scenario_denouement(rank, scenario_name, scenario):
 
         print("GEN: %4s PG:" % gen, end="")
 
+        previous_val = None
         for stage in stages:
             current_val = pyo.value(getattr(scenario, "stage_models_"+str(stage)).pg[gen])
-            if stage == stages[0]:
+            if previous_val is None:
                 print("%6.2f -->> " % current_val, end=" ")
             else:
                 print("%6.2f" % (current_val-previous_val), end=" ")
@@ -273,9 +271,10 @@ def scenario_denouement(rank, scenario_name, scenario):
 
         print("GEN: %4s QG:" % gen, end="")
 
+        previous_val = None
         for stage in stages:
             current_val = pyo.value(getattr(scenario, "stage_models_"+str(stage)).qg[gen])
-            if stage == stages[0]:
+            if previous_val is None:
                 print("%6.2f -->> " % current_val, end=" ")
             else:
                 print("%6.2f" % (current_val-previous_val), end=" ")

Diff for: examples/acopf3/fourstage.py

@@ -23,7 +23,7 @@
 # the problem
 import ACtree as etree
 from ccopf_multistage import pysp2_callback,\
-    scenario_denouement, _md_dict, FixFast, FixNever, FixGaussian
+    scenario_denouement, _md_dict, FixFast
 import rho_setter
 
 import pyomo.environ as pyo

Diff for: examples/aircond/aircond_cylinders.py

@@ -10,9 +10,6 @@
 # Use bundle_pickler.py to create bundle pickles
 # NOTE: As of 3 March 2022, you can't compare pickle bundle problems with non-pickled. See _demands_creator in aircondB.py for more discusion.
 
-import sys
-import os
-import copy
 import numpy as np
 import itertools
 from mpisppy import global_toc
@@ -101,7 +98,7 @@ def make_nodenames_balanced(BFs, leaf_nodes=False, root = True):
         0, 1, 2, ..., BFs[0], 0_0, 0_1, ..., 0_BFs[1], 1_0, 1_1, ... ,
         1_BFs[1], ... , BFs[0]_BFs[1], ... , BFs[0]...BFs[-2]
     """
-    if leaf_nodes == False:
+    if not leaf_nodes:
         BFs = BFs[:-1] # exclude leaf nodes
 
     # Constructs all nodenames
@@ -156,7 +153,7 @@ def _parse_args():
                          domain=bool,
                          default=False)    
     # special "proper" bundle arguments
-    parser = pickle_bundle.pickle_bundle_parser(cfg)
+    pickle_bundle.pickle_bundle_config(cfg)
 
     cfg.add_to_config("EF_directly",
                          description="Solve the EF directly instead of using cylinders (default False)",
@@ -211,15 +208,13 @@ def main():
         all_scenario_names = [f"Bundle_{bn*bsize}_{(bn+1)*bsize-1}" for bn in range(numbuns)]
         refmodule = aircondB
         primal_rho_setter = None
-        dual_rho_setter = None
         global_toc("WARNING: not using rho setters with proper bundles")
 
     else:
         ScenCount = np.prod(BFs)
         all_scenario_names = [f"scen{i}" for i in range(ScenCount)] #Scens are 0-based
         refmodule = aircond
         primal_rho_setter = refmodule.primal_rho_setter
-        dual_rho_setter = refmodule.dual_rho_setter
 
     xhat_scenario_dict = make_node_scenario_dict_balanced(BFs)
     all_nodenames = list(xhat_scenario_dict.keys())
@@ -243,8 +238,8 @@ def main():
         ama = amalgamator.from_module(refmodule,
                                       cfg, use_command_line=False)
         ama.run()
-        print(f"EF inner bound=", ama.best_inner_bound)
-        print(f"EF outer bound=", ama.best_outer_bound)
+        print("EF inner bound=", ama.best_inner_bound)
+        print("EF outer bound=", ama.best_outer_bound)
         quit()
 
     # if we are still here, we are running cylinders

Diff for: examples/aircond/aircond_ef.py

@@ -7,11 +7,9 @@
 # full copyright and license information.
 ###############################################################################
 # multistage (4-stage) example using aircond model. Can be any number of stages, does not support unbalanced trees
-import pyomo.environ as pyo
 import numpy as np
 import mpisppy.utils.sputils as sputils
 import mpisppy.utils.amalgamator as amalgamator
-from mpisppy import global_toc
 
 # Use this random stream:
 aircondstream = np.random.RandomState()
@@ -42,8 +40,8 @@
     ama = amalgamator.from_module(refmodel,
                                   ama_options,use_command_line=False)
     ama.run()
-    print(f"inner bound=", ama.best_inner_bound)
-    print(f"outer bound=", ama.best_outer_bound)
+    print("inner bound=", ama.best_inner_bound)
+    print("outer bound=", ama.best_outer_bound)
     print ("quitting early")
     quit()
     from mpisppy.confidence_intervals.mmw_ci import MMWConfidenceIntervals

Diff for: examples/aircond/aircond_seqsampling.py

@@ -9,14 +9,8 @@
 # Use the aircond model to illustrate how to use sequential sampling.
 #
 
-import sys
 import numpy as np
-import argparse
 import mpisppy.tests.examples.aircond as aircond
-import pyomo.environ as pyo
-import pyomo.common.config as pyofig
-import mpisppy.utils.sputils as sputils
-import mpisppy.utils.amalgamator as amalgamator
 import mpisppy.confidence_intervals.multi_seqsampling as multi_seqsampling
 import mpisppy.confidence_intervals.confidence_config as conf_config
 from mpisppy.utils import config
@@ -29,19 +23,10 @@ def main(cfg):
         results (dict): the solution, gap confidence interval and T 
     """
     refmodelname = "mpisppy.tests.examples.aircond"
-    scenario_creator = aircond.scenario_creator
 
     BFs = cfg.branching_factors
     num_scens = np.prod(BFs)
 
-    scenario_creator_kwargs = {"num_scens" : num_scens,
-                               "branching_factors": BFs,
-                               "mu_dev": cfg.mu_dev,
-                               "sigma_dev": cfg.sigma_dev,
-                               "start_ups": cfg.start_ups,
-                               "start_seed": cfg.seed,
-                               }
-    
     scenario_names = ['Scenario' + str(i) for i in range(num_scens)]
 
     xhat_gen_kwargs = {"scenario_names": scenario_names,
@@ -114,7 +99,7 @@ def _parse_args():
     if cfg.BM_vs_BPL is None:
         raise RuntimeError("--BM-vs-BPL must be given.")
     if cfg.BM_vs_BPL != "BM" and cfg.BM_vs_BPL != "BPL":
-        raise RuntimeError(f"--BM-vs-BPL must be BM or BPL (you gave {args.BM_vs_BMPL})")
+        raise RuntimeError(f"--BM-vs-BPL must be BM or BPL (you gave {cfg.BM_vs_BPL})")
 
     return cfg
 
@@ -124,7 +109,7 @@ def _parse_args():
     cfg.quick_assign("EF_mstage", bool, True)
 
     results = main(cfg)
-    print(f"Final gap confidence interval results:", results)
+    print("Final gap confidence interval results:", results)
 
     if cfg.xhat1_file is not None:
         print(f"Writing xhat1 to {cfg.xhat1_file}.npy")

Diff for: examples/aircond/bundle_pickler.py

@@ -11,11 +11,7 @@
 # see try_pickles.bash
 # parallel version
 
-import sys
-import os
-import copy
 import numpy as np
-import itertools
 import mpisppy.tests.examples.aircondB as aircondB
 from mpisppy.utils import config
 from mpisppy.utils import pickle_bundle
@@ -31,7 +27,7 @@
 def _parse_args():
     cfg = config.Config()
     cfg.multistage()
-    pickle_bundle.pickle_bundle_parser(cfg)
+    pickle_bundle.pickle_bundle_config(cfg)
     aircondB.inparser_adder(cfg)
     cfg.parse_command_line("bundle_pickler for aircond")
 
@@ -55,8 +51,6 @@ def main():
 
     bsize = int(cfg.scenarios_per_bundle)
     numbuns = ScenCount // bsize
-    # we won't actually use all names
-    all_bundle_names = [f"Bundle_{bn*bsize}_{(bn+1)*bsize-1}" for bn in range(numbuns)]
 
     if numbuns < n_proc:
         raise RuntimeError(