add formatting and linting python in pre-commit (#370)

* add ruff pre-commit * install ruff in pre-commit venv * add cmake target for ruff and ruff-format * fix formatting * fix linted warnings * be explicit about formatting options * target py311
key4hep · Sep 30, 2024 · eb8592e · eb8592e
1 parent c427374
commit eb8592e
Show file tree

Hide file tree

Showing 13 changed files with 240 additions and 204 deletions.
diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml
@@ -32,7 +32,7 @@ jobs:
           cd $STARTDIR
           python3 -m venv /root/pre-commit-venv
           source /root/pre-commit-venv/bin/activate
-          pip install pre-commit
+          pip install pre-commit ruff
           cmake -DENABLE_SIO=ON \
             -DCMAKE_CXX_STANDARD=20 \
             -DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always -Werror "\

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -18,3 +18,13 @@ repos:
         entry: python ./scripts/updateReadmeLinks.py --check
         files: (^README.md$)
         language: system
+      - id: ruff
+        name: ruff
+        entry: ruff check --force-exclude
+        types: [python]
+        language: system
+      - id: ruff-format
+        name: ruff-format
+        entry: ruff format --force-exclude
+        types: [python]
+        language: system
diff --git a/.ruff.toml b/.ruff.toml
@@ -0,0 +1,13 @@
+target-version = "py311"
+
+line-length = 99
+
+[format]
+# Make things format the same way as black
+quote-style = "double"
+indent-style = "space"
+skip-magic-trailing-comma = false
+line-ending = "auto"
+
+[lint]
+select = ["F", "E", "W", "PLE", "PLW", "PLC"]
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -73,3 +73,6 @@ add_subdirectory(test)
 
 #--- create uninstall target ---------------------------------------------------
 include(cmake/EDM4HEPUninstall.cmake)
+
+#--- code format targets -------------------------------------------------------
+include(cmake/pythonFormat.cmake)
diff --git a/cmake/pythonFormat.cmake b/cmake/pythonFormat.cmake
@@ -0,0 +1,25 @@
+# Additional target to run python linters and formatters on python files
+#
+# Requires ruff to be available in the environment
+
+# Get all our Python files
+file(GLOB_RECURSE ALL_PYTHON_FILES ${PROJECT_SOURCE_DIR}/python/*.py
+  ${PROJECT_SOURCE_DIR}/scripts/*.py ${PROJECT_SOURCE_DIR}/test/*.py)
+
+find_program(RUFF_EXECUTABLE ruff)
+if(RUFF_EXECUTABLE)
+    add_custom_target(
+        ruff
+        COMMAND ${RUFF_EXECUTABLE}
+        check --force-exclude ${ALL_PYTHON_FILES}
+    )
+    set_target_properties(ruff PROPERTIES EXCLUDE_FROM_ALL TRUE)
+    add_custom_target(
+        ruff-format
+        COMMAND ${RUFF_EXECUTABLE}
+        format --force-exclude ${ALL_PYTHON_FILES}
+    )
+    set_target_properties(ruff-format PROPERTIES EXCLUDE_FROM_ALL TRUE)
+else()
+    message(STATUS "Failed to find ruff executable - no target to run ruff can be set")
+endif()
diff --git a/python/edm4hep/__init__.py b/python/edm4hep/__init__.py
@@ -1,34 +1,37 @@
 """Python module for the EDM4HEP datamodel and utilities."""
+
 import sys
 
 from .__version__ import __version__
 import ROOT
-res = ROOT.gSystem.Load('libedm4hepDict')
+
+res = ROOT.gSystem.Load("libedm4hepDict")
 if res < 0:
-    raise RuntimeError('Failed to load libedm4hepDict')
+    raise RuntimeError("Failed to load libedm4hepDict")
 
-res = ROOT.gSystem.Load('libedm4hepRDF')
+res = ROOT.gSystem.Load("libedm4hepRDF")
 if res < 0:
-    raise RuntimeError('Failed to load libedm4hepRDF')
+    raise RuntimeError("Failed to load libedm4hepRDF")
 
-res = ROOT.gInterpreter.LoadFile('edm4hep/utils/kinematics.h')
+res = ROOT.gInterpreter.LoadFile("edm4hep/utils/kinematics.h")
 if res != 0:
-    raise RuntimeError('Failed to load kinematics.h')
+    raise RuntimeError("Failed to load kinematics.h")
 
-res = ROOT.gInterpreter.LoadFile('edm4hep/utils/dataframe.h')
+res = ROOT.gInterpreter.LoadFile("edm4hep/utils/dataframe.h")
 if res != 0:
-    raise RuntimeError('Failed to load dataframe.h')
+    raise RuntimeError("Failed to load dataframe.h")
 
-res = ROOT.gInterpreter.LoadFile('edm4hep/Constants.h')
+res = ROOT.gInterpreter.LoadFile("edm4hep/Constants.h")
 if res != 0:
-    raise RuntimeError('Failed to load Constants.h')
-from ROOT import edm4hep
+    raise RuntimeError("Failed to load Constants.h")
+from ROOT import edm4hep  # noqa: E402
+
+from podio.pythonizations import load_pythonizations  # noqa: E402
 
-from podio.pythonizations import load_pythonizations
-load_pythonizations('edm4hep')
+load_pythonizations("edm4hep")
 
 # Make TAB completion work for utils
-setattr(edm4hep, 'utils', edm4hep.utils)
+setattr(edm4hep, "utils", edm4hep.utils)
 
 # set package attributes for edm4hep
 edm4hep.__version__ = __version__
@@ -38,4 +41,4 @@
 edm4hep.__file__ = __file__
 
 # Make `import edm4hep` work
-sys.modules['edm4hep'] = edm4hep
+sys.modules["edm4hep"] = edm4hep
diff --git a/scripts/createEDM4hepFile.py b/scripts/createEDM4hepFile.py
@@ -45,15 +45,9 @@ def create_MCParticleCollection():
         particle.setCharge(next(counter))
         particle.setTime(next(counter))
         particle.setMass(next(counter))
-        particle.setVertex(
-            edm4hep.Vector3d(next(counter), next(counter), next(counter))
-        )
-        particle.setEndpoint(
-            edm4hep.Vector3d(next(counter), next(counter), next(counter))
-        )
-        particle.setMomentum(
-            edm4hep.Vector3d(next(counter), next(counter), next(counter))
-        )
+        particle.setVertex(edm4hep.Vector3d(next(counter), next(counter), next(counter)))
+        particle.setEndpoint(edm4hep.Vector3d(next(counter), next(counter), next(counter)))
+        particle.setMomentum(edm4hep.Vector3d(next(counter), next(counter), next(counter)))
         particle.setMomentumAtEndpoint(
             edm4hep.Vector3d(next(counter), next(counter), next(counter))
         )
@@ -174,9 +168,7 @@ def create_ClusterCollection(vectorsize, calo_hit):
     cluster.setPositionError(create_CovMatrixNf(3))
     cluster.setITheta(next(counter))
     cluster.setPhi(next(counter))
-    cluster.setDirectionError(
-        edm4hep.Vector3f(next(counter), next(counter), next(counter))
-    )
+    cluster.setDirectionError(edm4hep.Vector3f(next(counter), next(counter), next(counter)))
     for j in range(vectorsize):
         cluster.addToShapeParameters(next(counter))
         cluster.addToSubdetectorEnergies(next(counter))
@@ -256,9 +248,7 @@ def create_TrackCollection(vectorsize, tracker_hit):
         state.Z0 = next(counter)
         state.tanLambda = next(counter)
         state.time = next(counter)
-        state.referencePoint = edm4hep.Vector3f(
-            next(counter), next(counter), next(counter)
-        )
+        state.referencePoint = edm4hep.Vector3f(next(counter), next(counter), next(counter))
         state.covMatrix = create_CovMatrixNf(6)
         track.addToTrackStates(state)
 
@@ -292,9 +282,7 @@ def create_ReconstructedParticleCollection(vertex, cluster, track):
     rparticle.setPDG(next(counter))
     rparticle.setEnergy(next(counter))
     rparticle.setMomentum(edm4hep.Vector3f(next(counter), next(counter), next(counter)))
-    rparticle.setReferencePoint(
-        edm4hep.Vector3f(next(counter), next(counter), next(counter))
-    )
+    rparticle.setReferencePoint(edm4hep.Vector3f(next(counter), next(counter), next(counter)))
     rparticle.setCharge(next(counter))
     rparticle.setMass(next(counter))
     rparticle.setGoodnessOfPID(next(counter))
@@ -402,9 +390,7 @@ def create_frame():
     particles = frame.put(create_MCParticleCollection(), "MCParticleCollection")
     particle = particles[0]
 
-    hits = frame.put(
-        create_SimTrackerHitCollection(particle), "SimTrackerHitCollection"
-    )
+    hits = frame.put(create_SimTrackerHitCollection(particle), "SimTrackerHitCollection")
     simtracker_hit = hits[0]
 
     calo_contribs = frame.put(
@@ -422,9 +408,7 @@ def create_frame():
     hits = frame.put(create_CalorimeterHitCollection(), "CalorimeterHitCollection")
     calo_hit = hits[0]
 
-    clusters = frame.put(
-        create_ClusterCollection(VECTORSIZE, calo_hit), "ClusterCollection"
-    )
+    clusters = frame.put(create_ClusterCollection(VECTORSIZE, calo_hit), "ClusterCollection")
     cluster = clusters[0]
 
     hits = frame.put(create_TrackerHit3DCollection(), "TrackerHit3DCollection")
@@ -433,9 +417,7 @@ def create_frame():
 
     frame.put(create_RawTimeSeriesCollection(VECTORSIZE), "RawTimeSeriesCollection")
 
-    tracks = frame.put(
-        create_TrackCollection(VECTORSIZE, tracker_hit), "TrackCollection"
-    )
+    tracks = frame.put(create_TrackCollection(VECTORSIZE, tracker_hit), "TrackCollection")
     track = tracks[0]
 
     pids, pid = create_ParticleIDCollection(VECTORSIZE)
@@ -456,9 +438,7 @@ def create_frame():
 
     put_link_collection(frame, "RecoMCParticleLink", reco_particle, particle)
     put_link_collection(frame, "CaloHitSimCaloHitLink", calo_hit, simcalo_hit)
-    put_link_collection(
-        frame, "TrackerHitSimTrackerHitLink", tracker_hit, simtracker_hit
-    )
+    put_link_collection(frame, "TrackerHitSimTrackerHitLink", tracker_hit, simtracker_hit)
     put_link_collection(frame, "CaloHitMCParticleLink", calo_hit, particle)
     put_link_collection(frame, "ClusterMCParticleLink", cluster, particle)
     put_link_collection(frame, "TrackMCParticleLink", track, particle)
@@ -481,9 +461,7 @@ def create_frame():
     parser.add_argument(
         "--rntuple", action="store_true", help="Use a ROOT ntuple instead of EDM4hep"
     )
-    parser.add_argument(
-        "--output-file", type=str, help="Output file name", default="edm4hep.root"
-    )
+    parser.add_argument("--output-file", type=str, help="Output file name", default="edm4hep.root")
     args = parser.parse_args()
     output_file = args.output_file
 

diff --git a/scripts/updateReadmeLinks.py b/scripts/updateReadmeLinks.py
@@ -5,46 +5,70 @@
 
 parser = argparse.ArgumentParser()
 parser.add_argument("inputfile", help="The README file to run on", default="README.md")
-parser.add_argument("--check", action='store_true', help='Use if you want to just check the links without actually changing the file')
+parser.add_argument(
+    "--check",
+    action="store_true",
+    help="Use if you want to just check the links without actually changing the file",
+)
 args = parser.parse_args()
 
+
 def check_readme_links():
-  edm4hep_yaml_content = ""
-  with open("edm4hep.yaml") as edm4hep_yaml:
-    edm4hep_yaml_content = edm4hep_yaml.readlines()
-
-  readme_content = ""
-  with open("README.md", "r") as readme:
-    readme_content = readme.readlines()
-
-  new_readme_content = ""
-
-  links_are_ok = True
-  for readme_line in readme_content:
-    edm4hep_objects = re.findall("\[(.*?)\]\(https:\/\/github.com\/key4hep\/EDM4hep\/blob\/main\/edm4hep\.yaml#L(\d+?)\)", readme_line)
-    if edm4hep_objects:
-      for edm4hep_object, original_line_number in edm4hep_objects:
-        regex = f".*edm4hep::{edm4hep_object} *: *"
-        # find in which line it appears in edm4hep.yaml
-        edm4hep_yaml_line_numbers_with_match = [str(i + 1) for i, line in enumerate(edm4hep_yaml_content) if re.match(regex, line)]
-        if len(edm4hep_yaml_line_numbers_with_match) != 1:
-          print(f"Error: failed to replace line number for {edm4hep_object}, either no or several matches were found in edm4hep.yaml with the regex '{regex}'")
-          sys.exit(1)
-        if edm4hep_yaml_line_numbers_with_match[0] != original_line_number:
-          links_are_ok = False
-          readme_line = readme_line.replace(f"[{edm4hep_object}](https://github.com/key4hep/EDM4hep/blob/main/edm4hep.yaml#L{original_line_number})", f"[{edm4hep_object}](https://github.com/key4hep/EDM4hep/blob/main/edm4hep.yaml#L{edm4hep_yaml_line_numbers_with_match[0]})")
-          print(f"{edm4hep_object} is wrongly linked (line {original_line_number} --> {edm4hep_yaml_line_numbers_with_match[0]})")
-    new_readme_content += readme_line
-  return links_are_ok, new_readme_content
+    edm4hep_yaml_content = ""
+    with open("edm4hep.yaml") as edm4hep_yaml:
+        edm4hep_yaml_content = edm4hep_yaml.readlines()
+
+    readme_content = ""
+    with open("README.md", "r") as readme:
+        readme_content = readme.readlines()
+
+    new_readme_content = ""
+
+    links_are_ok = True
+    for readme_content_line in readme_content:
+        readme_line = readme_content_line
+        edm4hep_objects = re.findall(
+            r"\[(.*?)\]\(https:\/\/github.com\/key4hep\/EDM4hep\/blob\/main\/edm4hep\.yaml#L(\d+?)\)",
+            readme_line,
+        )
+        if edm4hep_objects:
+            for edm4hep_object, original_line_number in edm4hep_objects:
+                regex = f".*edm4hep::{edm4hep_object} *: *"
+                # find in which line it appears in edm4hep.yaml
+                edm4hep_yaml_line_numbers_with_match = [
+                    str(i + 1)
+                    for i, line in enumerate(edm4hep_yaml_content)
+                    if re.match(regex, line)
+                ]
+                if len(edm4hep_yaml_line_numbers_with_match) != 1:
+                    print(
+                        f"Error: failed to replace line number for {edm4hep_object}, either no "
+                        f"or several matches were found in edm4hep.yaml with the regex '{regex}'"
+                    )
+                    sys.exit(1)
+                if edm4hep_yaml_line_numbers_with_match[0] != original_line_number:
+                    links_are_ok = False
+                    readme_line = readme_line.replace(
+                        f"[{edm4hep_object}](https://github.com/key4hep/EDM4hep/blob/main/edm4hep.yaml#L{original_line_number})",
+                        f"[{edm4hep_object}](https://github.com/key4hep/EDM4hep/blob/main/edm4hep.yaml#L{edm4hep_yaml_line_numbers_with_match[0]})",
+                    )
+                    print(
+                        f"{edm4hep_object} is wrongly linked (line {original_line_number} --> {edm4hep_yaml_line_numbers_with_match[0]})"  # noqa: E501
+                    )
+        new_readme_content += readme_line
+    return links_are_ok, new_readme_content
+
 
 links_are_ok, new_readme_content = check_readme_links()
 if links_are_ok:
-  print("README.md links are fine, nothing to change")
+    print("README.md links are fine, nothing to change")
 else:
-  if not args.check:
-    with open("README.md", "w") as readme:
-      readme.write(new_readme_content)
-    print("README.md links updated.")
-  else:
-    print("README.md links should be updated. (Run ./scripts/updateReadmeLinks.py to fix them)")
-    sys.exit(1)
+    if not args.check:
+        with open("README.md", "w") as readme:
+            readme.write(new_readme_content)
+        print("README.md links updated.")
+    else:
+        print(
+            "README.md links should be updated. (Run ./scripts/updateReadmeLinks.py to fix them)"
+        )
+        sys.exit(1)
diff --git a/test/conftest.py b/test/conftest.py
@@ -14,5 +14,5 @@ def pytest_addoption(parser):
 def pytest_configure(config):
     """This is a slighty hacky solution to make the pytest configuration
     available in test modules outside of fixtures"""
-    global options
+    global options  # noqa: PLW0603
     options = config.option