From 3fe71407f5d19ebaa4f6be31fdba8015750d675a Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Sat, 27 May 2023 09:35:10 -0400
Subject: [PATCH 1/6] Add config files for ruff

---
 .pre-commit-config.yaml |   6 ++
 pyproject.toml          | 123 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 129 insertions(+)

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index f804b80..0b60216 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -4,3 +4,9 @@ repos:
     hooks:
     - id: black
       language_version: python3.10
+
+-   repo: https://github.com/charliermarsh/ruff-pre-commit
+    rev: v0.0.270
+    hooks:
+    -   id: ruff
+        args: [--exit-non-zero-on-fix]
diff --git a/pyproject.toml b/pyproject.toml
index 5d6a3d8..6de60bc 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -15,3 +15,126 @@ exclude = '''
   | docs
 )/
 '''
+
+# Copy ruff settings from pandas
+[tool.ruff]
+line-length = 100
+target-version = "py310"
+# fix = true
+unfixable = []
+
+select = [
+  # pyflakes
+  "F",
+  # pycodestyle
+  "E", "W",
+  # flake8-2020
+  "YTT",
+  # flake8-bugbear
+  "B",
+  # flake8-quotes
+  "Q",
+  # flake8-debugger
+  "T10",
+  # flake8-gettext
+  "INT",
+  # pylint
+  "PLC", "PLE", "PLR", "PLW",
+  # misc lints
+  "PIE",
+  # flake8-pyi
+  "PYI",
+  # tidy imports
+  "TID",
+  # implicit string concatenation
+  "ISC",
+  # type-checking imports
+  "TCH",
+  # comprehensions
+  "C4",
+  # pygrep-hooks
+  "PGH"
+]
+
+ignore = [
+  # space before : (needed for how black formats slicing)
+  # "E203",  # not yet implemented
+  # module level import not at top of file
+  "E402",
+  # do not assign a lambda expression, use a def
+  "E731",
+  # line break before binary operator
+  # "W503",  # not yet implemented
+  # line break after binary operator
+  # "W504",  # not yet implemented
+  # controversial
+  "B006",
+  # controversial
+  "B007",
+  # controversial
+  "B008",
+  # setattr is used to side-step mypy
+  "B009",
+  # getattr is used to side-step mypy
+  "B010",
+  # tests use assert False
+  "B011",
+  # tests use comparisons but not their returned value
+  "B015",
+  # false positives
+  "B019",
+  # Loop control variable overrides iterable it iterates
+  "B020",
+  # Function definition does not bind loop variable
+  "B023",
+  # Functions defined inside a loop must not use variables redefined in the loop
+  # "B301",  # not yet implemented
+  # Only works with python >=3.10
+  "B905",
+  # Too many arguments to function call
+  "PLR0913",
+  # Too many returns
+  "PLR0911",
+  # Too many branches
+  "PLR0912",
+  # Too many statements
+  "PLR0915",
+  # Redefined loop name
+  "PLW2901",
+  # Global statements are discouraged
+  "PLW0603",
+  # Docstrings should not be included in stubs
+  "PYI021",
+  # No builtin `eval()` allowed
+  "PGH001",
+  # compare-to-empty-string
+  "PLC1901",
+  # Use typing_extensions.TypeAlias for type aliases
+  # "PYI026",  # not yet implemented
+  # Use "collections.abc.*" instead of "typing.*" (PEP 585 syntax)
+  # "PYI027",  # not yet implemented
+  # while int | float can be shortened to float, the former is more explicit
+  # "PYI041",  # not yet implemented
+
+  # Additional checks that don't pass yet
+  # Useless statement
+  "B018",
+  # Within an except clause, raise exceptions with ...
+  "B904",
+  # Magic number
+  "PLR2004",
+  # Consider `elif` instead of `else` then `if` to remove indentation level
+  "PLR5501",
+]
+
+exclude = [
+  "doc/sphinxext/*.py",
+  "doc/build/*.py",
+  "doc/temp/*.py",
+  ".eggs/*.py",
+  # vendored files
+  "pandas/util/version/*",
+  "versioneer.py",
+  # exclude asv benchmark environments from linting
+  "env",
+]

From 9f0590997f5dc596d0c4263f586b992026a9af56 Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Sat, 27 May 2023 10:12:01 -0400
Subject: [PATCH 2/6] Update for ruff linting

---
 pyproject.toml                         |  4 +++
 sparkles/__init__.py                   |  2 +-
 sparkles/core.py                       | 36 ++++++++++++--------------
 sparkles/find_er_catalog.py            | 14 +++++-----
 sparkles/roll_optimize.py              | 15 +++++------
 sparkles/tests/test_checks.py          |  2 +-
 sparkles/tests/test_find_er_catalog.py | 13 +++++-----
 sparkles/tests/test_review.py          | 18 ++++++-------
 sparkles/tests/test_yoshi.py           |  8 +++---
 sparkles/yoshi.py                      | 34 ++++++++++++------------
 10 files changed, 71 insertions(+), 75 deletions(-)

diff --git a/pyproject.toml b/pyproject.toml
index 6de60bc..b3390b7 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -24,6 +24,8 @@ target-version = "py310"
 unfixable = []
 
 select = [
+  # isort
+  "I",
   # pyflakes
   "F",
   # pycodestyle
@@ -87,6 +89,8 @@ ignore = [
   "B020",
   # Function definition does not bind loop variable
   "B023",
+  # No explicit `stacklevel` keyword argument found
+  "B028",
   # Functions defined inside a loop must not use variables redefined in the loop
   # "B301",  # not yet implemented
   # Only works with python >=3.10
diff --git a/sparkles/__init__.py b/sparkles/__init__.py
index 51ac5c3..bede91e 100644
--- a/sparkles/__init__.py
+++ b/sparkles/__init__.py
@@ -2,7 +2,7 @@
 
 __version__ = ska_helpers.get_version(__package__)
 
-from .core import run_aca_review, ACAReviewTable  # noqa
+from .core import ACAReviewTable, run_aca_review  # noqa: F401
 
 
 def test(*args, **kwargs):
diff --git a/sparkles/core.py b/sparkles/core.py
index daea8c5..fd25fd3 100644
--- a/sparkles/core.py
+++ b/sparkles/core.py
@@ -6,34 +6,31 @@
 """
 import gzip
 import io
+import pickle
+import pprint
 import re
 import traceback
+from itertools import chain, combinations
 from pathlib import Path
-import pickle
-from itertools import combinations, chain
-import pprint
 
 import matplotlib
 
-matplotlib.use("Agg")  # noqa
+matplotlib.use("Agg")
+import chandra_aca
 import matplotlib.pyplot as plt
-from matplotlib.patches import Circle
-
 import numpy as np
-from jinja2 import Template
-from chandra_aca.star_probs import guide_count
-from chandra_aca.transform import yagzag_to_pixels, mag_to_count_rate, snr_mag_for_t_ccd
-import chandra_aca
-from astropy.table import Column, Table
-
 import proseco
-from proseco.catalog import ACATable
 import proseco.characteristics as ACA
+from astropy.table import Column, Table
+from chandra_aca.star_probs import guide_count
+from chandra_aca.transform import mag_to_count_rate, snr_mag_for_t_ccd, yagzag_to_pixels
+from jinja2 import Template
+from matplotlib.patches import Circle
+from proseco.catalog import ACATable
 from proseco.core import MetaAttribute
 
 from .roll_optimize import RollOptimizeMixin
 
-
 CACHE = {}
 FILEDIR = Path(__file__).parent
 
@@ -51,10 +48,10 @@
 
 def main(sys_args=None):
     """Command line interface to preview_load()"""
-    from . import __version__
-
     import argparse
 
+    from . import __version__
+
     parser = argparse.ArgumentParser(
         description=f"Sparkles ACA review tool {__version__}"
     )
@@ -514,7 +511,7 @@ def get_acas_from_pickle(load_name, loud=False):
     :param load_name: load name
     :param loud: print processing information
     """
-    if load_name.startswith(r"\\noodle") or load_name.startswith("https://occweb"):
+    if load_name.startswith((r"\\noodle", "https://occweb")):
         acas_dict, path_name = get_acas_dict_from_occweb(load_name)
     else:
         acas_dict, path_name = get_acas_dict_from_local_file(load_name, loud)
@@ -1171,7 +1168,7 @@ def check_guide_geometry(self):
         n_guide = len(guide_idxs)
 
         if n_guide < 2:
-            msg = f"Cannot check geometry with fewer than 2 guide stars"
+            msg = "Cannot check geometry with fewer than 2 guide stars"
             self.add_message("critical", msg)
             return
 
@@ -1574,7 +1571,8 @@ def from_ocat(cls, obsid, t_ccd=-5, man_angle=5, date=None, roll=None, **kwargs)
         :returns: AcaReviewTable object
         """
         from proseco import get_aca_catalog
-        from .yoshi import get_yoshi_params_from_ocat, convert_yoshi_to_proseco_params
+
+        from .yoshi import convert_yoshi_to_proseco_params, get_yoshi_params_from_ocat
 
         params_yoshi = get_yoshi_params_from_ocat(obsid, obs_date=date)
         if roll is not None:
diff --git a/sparkles/find_er_catalog.py b/sparkles/find_er_catalog.py
index edc7e33..999b322 100644
--- a/sparkles/find_er_catalog.py
+++ b/sparkles/find_er_catalog.py
@@ -63,17 +63,15 @@
      from the star fields that appear to be the best based on available stars.
    3. Prioritize by the order of attitudes passed in to the function.
 """
+import agasc
 import numpy as np
-from astropy.table import Table, MaskedColumn
-
-from chandra_aca.transform import radec_to_yagzag, snr_mag_for_t_ccd, yagzag_to_pixels
 import proseco.characteristics_guide as GUIDE
+import ska_sun
+from astropy.table import MaskedColumn, Table
+from chandra_aca.star_probs import guide_count
+from chandra_aca.transform import radec_to_yagzag, snr_mag_for_t_ccd, yagzag_to_pixels
 from proseco import get_aca_catalog
-import agasc
 from Quaternion import Quat
-from chandra_aca.star_probs import guide_count
-import ska_sun
-
 from ska_sun import get_sun_pitch_yaw
 
 
@@ -317,7 +315,7 @@ def get_att_opts_table(acar, atts):
     )
     # Creating a numpy object array of empty lists requires this workaround
     for ii in range(n_atts):
-        t["status"][ii] = list()
+        t["status"][ii] = []
 
     for name in ["dpitch", "dyaw", "droll", "count_9th", "count_10th", "count_all"]:
         t[name].info.format = ".2f"
diff --git a/sparkles/roll_optimize.py b/sparkles/roll_optimize.py
index 779004e..db516da 100644
--- a/sparkles/roll_optimize.py
+++ b/sparkles/roll_optimize.py
@@ -4,23 +4,22 @@
 """
 Roll optimization during preliminary review of ACA catalogs.
 """
-from copy import deepcopy
-import numpy as np
 import warnings
+from copy import deepcopy
 
+import numpy as np
+import ska_sun
 from astropy.table import Table, vstack
 from chandra_aca.star_probs import acq_success_prob, guide_count
 from chandra_aca.transform import (
-    radec_to_yagzag,
-    yagzag_to_pixels,
     calc_aca_from_targ,
     calc_targ_from_aca,
+    radec_to_yagzag,
+    yagzag_to_pixels,
 )
-from Quaternion import Quat
-import ska_sun
-
-from proseco.characteristics import CCD
 from proseco import get_aca_catalog
+from proseco.characteristics import CCD
+from Quaternion import Quat
 
 
 def logical_intervals(vals, x=None):
diff --git a/sparkles/tests/test_checks.py b/sparkles/tests/test_checks.py
index a8d2b48..cb6e661 100644
--- a/sparkles/tests/test_checks.py
+++ b/sparkles/tests/test_checks.py
@@ -13,7 +13,7 @@
 from proseco.tests.test_common import DARK40, STD_INFO, mod_std_info
 from Quaternion import Quat
 
-from .. import ACAReviewTable
+from sparkles import ACAReviewTable
 
 
 def test_check_slice_index():
diff --git a/sparkles/tests/test_find_er_catalog.py b/sparkles/tests/test_find_er_catalog.py
index 3a1ddcb..8f95e7f 100644
--- a/sparkles/tests/test_find_er_catalog.py
+++ b/sparkles/tests/test_find_er_catalog.py
@@ -1,26 +1,25 @@
 # Licensed under a 3-clause BSD style license - see LICENSE.rst
 import os
-
 import warnings
+
+import agasc
+import numpy as np
+import Ska.Sun
 from proseco import get_aca_catalog
 from proseco.tests.test_common import mod_std_info
-import numpy as np
 from Quaternion import Quat
-import Ska.Sun
-import agasc
 
 # Do not use the AGASC supplement in testing since mags can change
 os.environ[agasc.SUPPLEMENT_ENABLED_ENV] = "False"
 
 
 from sparkles.find_er_catalog import (
-    get_candidate_stars,
-    find_er_catalog,
     filter_candidate_stars_on_ccd,
+    find_er_catalog,
+    get_candidate_stars,
     get_guide_counts,
 )
 
-
 # Known tough field: PKS 0023-26 pointing
 ATT = Quat([0.20668099834, 0.23164729391, 0.002658888173, 0.9505868852])
 DATE = "2021-09-13"
diff --git a/sparkles/tests/test_review.py b/sparkles/tests/test_review.py
index cbb598a..e923970 100644
--- a/sparkles/tests/test_review.py
+++ b/sparkles/tests/test_review.py
@@ -1,22 +1,20 @@
-import os
-import numpy as np
 import gzip
+import os
 import pickle
 from pathlib import Path
-from proseco.core import StarsTable
 
+import agasc
+import numpy as np
 import pytest
+import ska_sun
 from proseco import get_aca_catalog
-from proseco.characteristics import aca_t_ccd_penalty_limit, MonFunc, MonCoord
-
-import agasc
+from proseco.characteristics import MonCoord, MonFunc, aca_t_ccd_penalty_limit
+from proseco.core import StarsTable
+from proseco.tests.test_common import DARK40, mod_std_info
 from Quaternion import Quat
-import ska_sun
 from ska_sun import apply_sun_pitch_yaw, nominal_roll
 
-from proseco.tests.test_common import DARK40, mod_std_info
-
-from .. import ACAReviewTable, run_aca_review
+from sparkles import ACAReviewTable, run_aca_review
 
 # Do not use the AGASC supplement in testing by default since mags can change
 os.environ[agasc.SUPPLEMENT_ENABLED_ENV] = "False"
diff --git a/sparkles/tests/test_yoshi.py b/sparkles/tests/test_yoshi.py
index d60f1ab..9b4b019 100644
--- a/sparkles/tests/test_yoshi.py
+++ b/sparkles/tests/test_yoshi.py
@@ -1,14 +1,14 @@
-import numpy as np
 import agasc
+import numpy as np
 import pytest
-from Quaternion import Quat
 from mica.archive.tests.test_cda import HAS_WEB_SERVICES
-from sparkles.core import ACAReviewTable
+from Quaternion import Quat
 
+from sparkles.core import ACAReviewTable
 from sparkles.yoshi import (
+    convert_yoshi_to_proseco_params,
     get_yoshi_params_from_ocat,
     run_one_yoshi,
-    convert_yoshi_to_proseco_params,
 )
 
 
diff --git a/sparkles/yoshi.py b/sparkles/yoshi.py
index 5a3f60f..90a5615 100644
--- a/sparkles/yoshi.py
+++ b/sparkles/yoshi.py
@@ -1,10 +1,10 @@
 import numpy as np
 import numpy.ma
-from chandra_aca.transform import calc_aca_from_targ
 from chandra_aca.drift import get_aca_offsets, get_target_aimpoint
-from mica.archive.cda import get_ocat_web, get_ocat_local
-from ska_sun import nominal_roll
+from chandra_aca.transform import calc_aca_from_targ
 from cxotime import CxoTime
+from mica.archive.cda import get_ocat_local, get_ocat_web
+from ska_sun import nominal_roll
 
 
 def get_yoshi_params_from_ocat(obsid, obs_date=None, web_ocat=True, cycle=None):
@@ -245,20 +245,20 @@ def convert_yoshi_to_proseco_params(
     )
 
     # Get keywords for proseco
-    out = dict(
-        obsid=obsid,
-        att=q_aca,
-        man_angle=man_angle,
-        date=obs_date,
-        t_ccd=t_ccd,
-        dither=(dither_y, dither_z),
-        detector=detector,
-        sim_offset=sim_offset,
-        focus_offset=focus_offset,
-        n_acq=8,
-        n_guide=5,
-        n_fid=3,
-    )
+    out = {
+        "obsid": obsid,
+        "att": q_aca,
+        "man_angle": man_angle,
+        "date": obs_date,
+        "t_ccd": t_ccd,
+        "dither": (dither_y, dither_z),
+        "detector": detector,
+        "sim_offset": sim_offset,
+        "focus_offset": focus_offset,
+        "n_acq": 8,
+        "n_guide": 5,
+        "n_fid": 3,
+    }
 
     out.update(kwargs)
 

From 5bdf741d0af1f86e3baf5fb71793e30ae5863d08 Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Sat, 27 May 2023 10:13:10 -0400
Subject: [PATCH 3/6] Add black/ruff linting workflow, remove flake8

---
 .github/workflows/flake8.yml         | 19 -------------------
 .github/workflows/python-linting.yml | 11 +++++++++++
 2 files changed, 11 insertions(+), 19 deletions(-)
 delete mode 100644 .github/workflows/flake8.yml
 create mode 100644 .github/workflows/python-linting.yml

diff --git a/.github/workflows/flake8.yml b/.github/workflows/flake8.yml
deleted file mode 100644
index d2c0951..0000000
--- a/.github/workflows/flake8.yml
+++ /dev/null
@@ -1,19 +0,0 @@
-name: Python flake8 check
-
-on: [push]
-
-jobs:
-  build:
-
-    runs-on: ubuntu-latest
-
-    steps:
-    - uses: actions/checkout@v3
-    - name: Set up Python 3.10
-      uses: actions/setup-python@v4
-      with:
-        python-version: "3.10"
-    - name: Lint with flake8
-      run: |
-        pip install flake8
-        flake8 . --count --ignore=W503,E402,F541,E203 --max-line-length=100 --show-source --statistics
diff --git a/.github/workflows/python-linting.yml b/.github/workflows/python-linting.yml
new file mode 100644
index 0000000..2c5fcca
--- /dev/null
+++ b/.github/workflows/python-linting.yml
@@ -0,0 +1,11 @@
+name: Check Python formatting using Black and Ruff
+
+on: [push]
+
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: psf/black@stable
+      - uses: chartboost/ruff-action@v1

From 660da32036d8c96fbfb2653e9e6646bf3dae215e Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Sat, 27 May 2023 10:15:48 -0400
Subject: [PATCH 4/6] Black on setup.py

---
 setup.py | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/setup.py b/setup.py
index 64eaa78..1226bb5 100644
--- a/setup.py
+++ b/setup.py
@@ -5,19 +5,22 @@
 except ImportError:
     cmdclass = {}
 
-entry_points = {'console_scripts': ['sparkles=sparkles.core:main']}
+entry_points = {"console_scripts": ["sparkles=sparkles.core:main"]}
 
-setup(name='sparkles',
-      author='Tom Aldcroft',
-      description='Sparkles ACA review package',
-      author_email='taldcroft@cfa.harvard.edu',
-      use_scm_version=True,
-      setup_requires=['setuptools_scm', 'setuptools_scm_git_archive'],
-      zip_safe=False,
-      entry_points=entry_points,
-      packages=['sparkles', 'sparkles.tests'],
-      package_data={'sparkles': ['index_template*.html'],
-                    'sparkles.tests': ['data/*pkl.gz']},
-      tests_require=['pytest'],
-      cmdclass=cmdclass,
-      )
+setup(
+    name="sparkles",
+    author="Tom Aldcroft",
+    description="Sparkles ACA review package",
+    author_email="taldcroft@cfa.harvard.edu",
+    use_scm_version=True,
+    setup_requires=["setuptools_scm", "setuptools_scm_git_archive"],
+    zip_safe=False,
+    entry_points=entry_points,
+    packages=["sparkles", "sparkles.tests"],
+    package_data={
+        "sparkles": ["index_template*.html"],
+        "sparkles.tests": ["data/*pkl.gz"],
+    },
+    tests_require=["pytest"],
+    cmdclass=cmdclass,
+)

From 86620ee55bc6396b1a135a60657e1379392583bb Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Sat, 27 May 2023 10:19:36 -0400
Subject: [PATCH 5/6] Fix pandas-specific config

---
 pyproject.toml | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/pyproject.toml b/pyproject.toml
index b3390b7..7b934ba 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -132,13 +132,5 @@ ignore = [
 ]
 
 exclude = [
-  "doc/sphinxext/*.py",
-  "doc/build/*.py",
-  "doc/temp/*.py",
-  ".eggs/*.py",
-  # vendored files
-  "pandas/util/version/*",
-  "versioneer.py",
-  # exclude asv benchmark environments from linting
-  "env",
+  "docs/",
 ]

From f5d7f957b22182eeaf19fe2d885d446b913634ca Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Sat, 27 May 2023 10:36:47 -0400
Subject: [PATCH 6/6] Flag loop control variable not used in loop

---
 pyproject.toml            | 4 ++--
 sparkles/roll_optimize.py | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/pyproject.toml b/pyproject.toml
index 7b934ba..fe58854 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -71,8 +71,8 @@ ignore = [
   # "W504",  # not yet implemented
   # controversial
   "B006",
-  # controversial
-  "B007",
+  # controversial?: Loop control variable not used within loop body
+  # "B007",
   # controversial
   "B008",
   # setattr is used to side-step mypy
diff --git a/sparkles/roll_optimize.py b/sparkles/roll_optimize.py
index db516da..73b1df3 100644
--- a/sparkles/roll_optimize.py
+++ b/sparkles/roll_optimize.py
@@ -168,7 +168,7 @@ def get_roll_intervals(
         def get_ids_list(roll_offsets):
             ids_list = []
 
-            for ii, roll_offset in enumerate(roll_offsets):
+            for roll_offset in roll_offsets:
                 # Roll about the target attitude, which is offset from ACA attitude by a bit
                 att_targ_rolled = Quat(
                     [att_targ.ra, att_targ.dec, att_targ.roll + roll_offset]