Merge pull request #2652 from MRtrix3/clangformat

Apply formatting for C++ using clang-format

.clang-format

@@ -0,0 +1,6 @@
+Language: Cpp
+BasedOnStyle: LLVM
+BinPackArguments: false
+BinPackParameters: false
+PackConstructorInitializers: NextLine
+ColumnLimit: 120

.git-blame-ignore-revs

@@ -97,3 +97,8 @@ aad44d847ac48d02bb7f8badf801dbfaa0ccdac0
 #Author: MRtrixBot <[email protected]>
 #Date:   Tue Jan 3 13:42:58 2023 +0100
 #    Update Copyright notice in command docs
+
+45fc06bdf687584bf4d55140720e84c5ef517bf0
+#Author: MRtrixBot <[email protected]>
+#Date:   Thu, 17 Aug 2023 15:36:14 +0100
+#    Apply formatting for C++ using clang-format

.github/workflows/checks.yml

@@ -214,3 +214,16 @@ jobs:
 
     - name: check building of documentation
       run: python3 -m sphinx -n -N -W -w sphinx.log docs/ tmp/
+
+  clang-format-check:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v1
+
+    - name: clang-format
+      uses: jidicula/[email protected]
+      with:
+        clang-format-version: '16'
+        fallback-style: 'LLVM'
+        # Ignore third-party headers.
+        exclude-regex: 'core/file/(json|nifti[12]).h'

.pre-commit-config.yaml

@@ -0,0 +1,10 @@
+default_language_version:
+  python: python3
+repos:
+  - repo: https://github.com/pre-commit/mirrors-clang-format
+    rev: v16.0.4
+    hooks:
+      - id: clang-format
+        name: clang-format
+        types_or: [c++]
+        language: python

CONTRIBUTING.md

@@ -307,6 +307,33 @@ A few explicit notes on such:
    introduce Windows-style newline characters ("`CR LF`" / "`\r\n`")
    will need to be edited accordingly.
 
+-  In C++, we use [clang-format](https://clang.llvm.org/docs/ClangFormat.html) 16
+   to ensure that all C++ code is formatted using the same conventions.
+   To comply with this requirement, we recommend that you install a local git hook
+   using [pre-commit](https://pre-commit.com/):
+
+   - Install `pre-commit` via `pip install pre-commit` (or via your OS package manager).
+
+   - Run `pre-commit install` in the source directory of MRtrix. 
+     NOTE: you may need to add the pip scripts folder to your `PATH` 
+     (you can check where pip has installed the package using `pip show pre-commit`).
+
+   This procedure will install a local git hook to ensure that all of your commits
+   are correctly formatted. If this isn't the case, the commits will fail and an 
+   automatic formatting of the staged changes will be applied. You will then have 
+   to commit those changes again.
+
+   Alternatively, you may wish to configure your editor to automatically format 
+   your code using `clang-format`. We also provide a Python script in the top level directory
+   called `clang-format-all` that can format all C++ code in the repository. You can run
+   this to format your code before committing it. The script also allows you to
+   specify a custom `clang-format` binary by using the `--executable` flag.
+
+   NOTE: If you are using a version of `clang-format` that is different from the one
+   used by the CI (currently version 16), your code may be formatted differently and thus
+   fail our CI tests. We recommend that you use the same version of `clang-format` as the
+   CI to avoid this issue.
+
 -  In Python, variable / class / module names are enforced through
    `pylint`. Script "`run_pylint`" in the *MRtrix3* root directory
    will test any code modifications against these expectations.

clang-format-all

@@ -0,0 +1,85 @@
+#!/usr/env/python3
+
+# Copyright (c) 2008-2023 the MRtrix3 contributors.
+#
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# Covered Software is provided under this License on an "as is"
+# basis, without warranty of any kind, either expressed, implied, or
+# statutory, including, without limitation, warranties that the
+# Covered Software is free of defects, merchantable, fit for a
+# particular purpose or non-infringing.
+# See the Mozilla Public License v. 2.0 for more details.
+#
+# For more details, see http://www.mrtrix.org/.
+
+import os
+import argparse
+import subprocess
+import time
+from pathlib import Path
+from multiprocessing import cpu_count
+
+parser = argparse.ArgumentParser(
+    description='Run clang-format on C++ source files.'
+)
+
+parser.add_argument(
+    '-e', '--executable',
+    help='Path to the clang-format executable.',
+    default='clang-format'
+)
+
+args = parser.parse_args()
+clang_format = args.executable
+paths = ['core', 'cmd', 'src', 'testing']
+extensions = ['.h', '.cpp']
+exclusion_list = ['core/file/nifti1.h',
+                  'core/file/nifti2.h',
+                  'core/file/json.h']
+
+# if clang-format path contains spaces, wrap it in quotes
+if ' ' in clang_format and not clang_format.startswith('"'):
+    clang_format = '"{}"'.format(clang_format)
+
+if os.system('{} -version'.format(clang_format)) != 0:
+    raise RuntimeError('Could not find clang-format executable.')
+
+
+def skip_file(file):
+    """Return True if the file should be skipped."""
+    return file.suffix not in extensions or file.as_posix() in exclusion_list
+
+
+def format_file(file):
+    """Run clang-format on a file."""
+    command = '{} -i {}'.format(clang_format, file)
+    return subprocess.Popen(command, shell=True)
+
+
+files = []
+for path in paths:
+    files += [file for file in list(Path(path).rglob('*.*'))
+              if not skip_file(file)]
+
+print('Found {} files to format.'.format(len(files)))
+
+# We want a maximum of num_cpus processes running at once
+processes = []
+num_cpus = cpu_count()
+total = len(files)
+
+try:
+    while len(files) > 0 or len(processes) > 0:
+        processes = [proc for proc in processes if proc.poll() is None]
+        schedule_count = min(num_cpus - len(processes), len(files))
+        processes += [format_file(files.pop(0)) for _ in range(schedule_count)]
+        print('Formatted {}/{} files.'.format(total - len(files), total), end='\r')
+        time.sleep(0.01)
+
+except KeyboardInterrupt:
+    print('Keyboard interrupt received, terminating formatting.')
+    for process in processes:
+        process.terminate()

cmd/5tt2gmwmi.cpp

@@ -25,110 +25,100 @@
 #include "dwi/tractography/ACT/act.h"
 #include "dwi/tractography/ACT/tissues.h"
 
-
-
 using namespace MR;
 using namespace App;
 
-
-void usage ()
-{
+void usage() {
 
   AUTHOR = "Robert E. Smith ([email protected])";
 
   SYNOPSIS = "Generate a mask image appropriate for seeding streamlines on the grey matter-white matter interface";
 
   REFERENCES
-    + "Smith, R. E.; Tournier, J.-D.; Calamante, F. & Connelly, A. " // Internal
-      "Anatomically-constrained tractography:"
-      "Improved diffusion MRI streamlines tractography through effective use of anatomical information. "
-      "NeuroImage, 2012, 62, 1924-1938";
+  +"Smith, R. E.; Tournier, J.-D.; Calamante, F. & Connelly, A. " // Internal
+   "Anatomically-constrained tractography:"
+   "Improved diffusion MRI streamlines tractography through effective use of anatomical information. "
+   "NeuroImage, 2012, 62, 1924-1938";
 
   ARGUMENTS
-    + Argument ("5tt_in",  "the input 5TT segmented anatomical image").type_image_in()
-    + Argument ("mask_out", "the output mask image")                  .type_image_out();
+  +Argument("5tt_in", "the input 5TT segmented anatomical image").type_image_in() +
+      Argument("mask_out", "the output mask image").type_image_out();
 
   OPTIONS
-    + Option("mask_in", "Filter an input mask image according to those voxels that lie upon the grey matter - white matter boundary. "
-                        "If no input mask is provided, the output will be a whole-brain mask image calculated using the anatomical image only.")
-      + Argument ("image", "the input mask image").type_image_in();
-
+  +Option("mask_in",
+          "Filter an input mask image according to those voxels that lie upon the grey matter - white matter boundary. "
+          "If no input mask is provided, the output will be a whole-brain mask image calculated using the anatomical "
+          "image only.") +
+      Argument("image", "the input mask image").type_image_in();
 }
 
+class Processor {
 
+public:
+  Processor(const Image<bool> &mask) : mask(mask) {}
+  Processor(const Processor &) = default;
 
-class Processor
-{ 
-
-  public:
-    Processor (const Image<bool>& mask) : mask (mask) { }
-    Processor (const Processor&) = default;
-
-    bool operator() (Image<float>& input, Image<float>& output)
-    {
-      // If a mask is defined, but is false in this voxel, do not continue processing
-      bool process_voxel = true;
-      if (mask.valid()) {
-        assign_pos_of (input, 0, 3).to (mask);
-        process_voxel = mask.value();
-      }
-      if (process_voxel) {
-        // Generate a non-binary seeding mask.
-        // Image intensity should be proportional to the tissue gradient present across the voxel
-        // Remember: This seeding mask is generated in the same space as the 5TT image: exploit this
-        //   (no interpolators should be necessary)
-        // Essentially looking for an absolute gradient in (GM - WM) - just do in three axes
-        //   - well, not quite; needs to be the minimum of the two
-        default_type gradient = 0.0;
-        for (size_t axis = 0; axis != 3; ++axis) {
-          assign_pos_of (output, 0, 3).to (input);
-          default_type multiplier = 0.5;
-          if (!output.index(axis)) {
-            multiplier = 1.0;
-          } else {
-            input.move_index (axis, -1);
-          }
-          const DWI::Tractography::ACT::Tissues neg (input);
-          if (output.index(axis) == output.size(axis)-1) {
-            multiplier = 1.0;
-            input.index(axis) = output.index(axis);
-          } else {
-            input.index(axis) = output.index(axis) + 1;
-          }
-          const DWI::Tractography::ACT::Tissues pos (input);
-          gradient += Math::pow2 (multiplier * std::min (abs (pos.get_gm() - neg.get_gm()), abs (pos.get_wm() - neg.get_wm())));
+  bool operator()(Image<float> &input, Image<float> &output) {
+    // If a mask is defined, but is false in this voxel, do not continue processing
+    bool process_voxel = true;
+    if (mask.valid()) {
+      assign_pos_of(input, 0, 3).to(mask);
+      process_voxel = mask.value();
+    }
+    if (process_voxel) {
+      // Generate a non-binary seeding mask.
+      // Image intensity should be proportional to the tissue gradient present across the voxel
+      // Remember: This seeding mask is generated in the same space as the 5TT image: exploit this
+      //   (no interpolators should be necessary)
+      // Essentially looking for an absolute gradient in (GM - WM) - just do in three axes
+      //   - well, not quite; needs to be the minimum of the two
+      default_type gradient = 0.0;
+      for (size_t axis = 0; axis != 3; ++axis) {
+        assign_pos_of(output, 0, 3).to(input);
+        default_type multiplier = 0.5;
+        if (!output.index(axis)) {
+          multiplier = 1.0;
+        } else {
+          input.move_index(axis, -1);
         }
-        output.value() = std::max (0.0, std::sqrt (gradient));
-        assign_pos_of (output, 0, 3).to (input);
-      } else {
-        output.value() = 0.0f;
+        const DWI::Tractography::ACT::Tissues neg(input);
+        if (output.index(axis) == output.size(axis) - 1) {
+          multiplier = 1.0;
+          input.index(axis) = output.index(axis);
+        } else {
+          input.index(axis) = output.index(axis) + 1;
+        }
+        const DWI::Tractography::ACT::Tissues pos(input);
+        gradient +=
+            Math::pow2(multiplier * std::min(abs(pos.get_gm() - neg.get_gm()), abs(pos.get_wm() - neg.get_wm())));
       }
-      return true;
+      output.value() = std::max(0.0, std::sqrt(gradient));
+      assign_pos_of(output, 0, 3).to(input);
+    } else {
+      output.value() = 0.0f;
     }
+    return true;
+  }
 
-  private:
-    Image<bool> mask;
-
+private:
+  Image<bool> mask;
 };
 
+void run() {
 
-
-void run ()
-{
-
-  auto input = Image<float>::open (argument[0]);
-  DWI::Tractography::ACT::verify_5TT_image (input);
-  check_3D_nonunity (input);
+  auto input = Image<float>::open(argument[0]);
+  DWI::Tractography::ACT::verify_5TT_image(input);
+  check_3D_nonunity(input);
 
   // TODO It would be nice to have the capability to define this mask based on another image
   // This will however require the use of interpolators
 
   Image<bool> mask;
-  auto opt = get_options ("mask_in");
+  auto opt = get_options("mask_in");
   if (opt.size()) {
-    mask = Image<bool>::open (opt[0][0]);
-    if (!dimensions_match (input, mask, 0, 3))
-      throw Exception ("Mask image provided using the -mask option must match the input 5TT image");
+    mask = Image<bool>::open(opt[0][0]);
+    if (!dimensions_match(input, mask, 0, 3))
+      throw Exception("Mask image provided using the -mask option must match the input 5TT image");
   }
 
   Header H;
@@ -140,10 +130,7 @@ void run ()
     H = input;
     H.ndim() = 3;
   }
-  auto output = Image<float>::create (argument[1], H);
-
-  ThreadedLoop ("Generating GMWMI seed mask", input, 0, 3)
-    .run (Processor (mask), input, output);
+  auto output = Image<float>::create(argument[1], H);
 
+  ThreadedLoop("Generating GMWMI seed mask", input, 0, 3).run(Processor(mask), input, output);
 }
-