Add in a C++ linter (#555)

.github/workflows/pynorms.yaml → .github/workflows/norms.yaml

@@ -1,10 +1,10 @@
-name: Python Coding Norms
+name: Coding Norms
 on: [push]
 
 jobs:
   check_pynorms:
     runs-on: ubuntu-latest
-    name: Check Python coding norms with pycodestyle
+    name: Check coding norms with pycodestyle and cpplint
 
     steps:
 
@@ -20,4 +20,9 @@ jobs:
     - name: Run pycodestyle
       run: |
         cd $GITHUB_WORKSPACE/GDASApp
-        pycodestyle -v --config ./.pycodestyle .
+        pycodestyle -v --config ./.pycodestyle ./ush ./scripts ./test 
+
+    - name: Run C++ linter on utils 
+      run: |
+        cd $GITHUB_WORKSPACE/GDASApp/utils/test/
+        ./cpplint.py --quiet --recursive $GITHUB_WORKSPACE/GDASApp/utils

utils/CPPLINT.cfg

@@ -0,0 +1,3 @@
+set noparent
+linelength=100
+filter=+build,-legal,+readability,+runtime,+whitespace,-runtime/references,-runtime/printf,-build/include_subdir

utils/ioda_example/gdas_meanioda.h

@@ -1,5 +1,9 @@
+#pragma once
+
 #include <iostream>
 #include <numeric>
+#include <string>
+#include <vector>
 #include "eckit/config/LocalConfiguration.h"
 #include "ioda/Group.h"
 #include "ioda/ObsSpace.h"
@@ -26,11 +30,10 @@ namespace gdasapp {
     static const std::string classname() {return "gdasapp::IodaExample";}
 
     int execute(const eckit::Configuration & fullConfig, bool /*validate*/) const {
-
       // get the obs space configuration
       const eckit::LocalConfiguration obsConfig(fullConfig, "obs space");
       ioda::ObsTopLevelParameters obsparams;
-      obsparams.validateAndDeserialize(obsConfig);  // TODO CRM, can I remove this and then the simulated vars junk??
+      obsparams.validateAndDeserialize(obsConfig);
       oops::Log::info() << "obs space: " << std::endl << obsConfig << std::endl;
 
       // time window stuff
@@ -51,13 +54,16 @@ namespace gdasapp {
 
       // read the obs space
       // Note, the below line does a lot of heavy lifting
-      // we can probably go to a lower level function (and more of them) to accomplish the same thing
-      ioda::ObsSpace ospace(obsparams, oops::mpi::world(), util::DateTime(winbegin), util::DateTime(winend), oops::mpi::myself());
+      // we can probably go to a lower level function
+      // (and more of them) to accomplish the same thing
+      ioda::ObsSpace ospace(obsparams, oops::mpi::world(), util::DateTime(winbegin),
+                            util::DateTime(winend), oops::mpi::myself());
       const size_t nlocs = ospace.nlocs();
       oops::Log::info() << "nlocs =" << nlocs << std::endl;
       std::vector<float> buffer(nlocs);
 
-      // below is grabbing from the IODA obs space the specified group/variable and putting it into the buffer
+      // below is grabbing from the IODA obs space
+      // the specified group/variable and putting it into the buffer
       if (chan == 0) {
         // no channel is selected
         ospace.get_db(group, variable, buffer);
@@ -67,12 +73,15 @@ namespace gdasapp {
       }
 
       // the below line computes the mean, aka sum divided by count
-      const float mean = std::accumulate(buffer.begin(), buffer.end(), 0) / float(nlocs);
+      const float mean = std::accumulate(buffer.begin(), buffer.end(), 0) /
+                         static_cast<float>(nlocs);
 
       // write the mean out to the stdout
-      oops::Log::info() << "mean value for " << group << "/" << variable << "=" << mean << std::endl;
+      oops::Log::info() << "mean value for " << group << "/" << variable
+                        << "=" << mean << std::endl;
 
-      // a better program should return a real exit code depending on result, but this is just an example!
+      // a better program should return a real exit code depending on result,
+      // but this is just an example!
       return 0;
     }
     // -----------------------------------------------------------------------------

utils/soca/gdas_incr_handler.h

@@ -1,6 +1,10 @@
+#pragma once
+
 #include <netcdf>
-#include <iostream>
+
 #include <filesystem>
+#include <iostream>
+#include <string>
 
 #include "eckit/config/LocalConfiguration.h"
 
@@ -18,7 +22,7 @@
 #include "soca/LinearVariableChange/LinearVariableChange.h"
 #include "soca/State/State.h"
 
-# include "gdas_postprocincr.h"
+#include "gdas_postprocincr.h"
 
 namespace gdasapp {
 
@@ -29,7 +33,6 @@ namespace gdasapp {
     static const std::string classname() {return "gdasapp::SocaIncrHandler";}
 
     int execute(const eckit::Configuration & fullConfig, bool /*validate*/) const {
-
       /// Setup the soca geometry
       const eckit::LocalConfiguration geomConfig(fullConfig, "geometry");
       oops::Log::info() << "geometry: " << std::endl << geomConfig << std::endl;
@@ -39,7 +42,8 @@ namespace gdasapp {
       // Initialize the post processing
       PostProcIncr postProcIncr(fullConfig, geom, this->getComm());
 
-      oops::Log::info() << "soca increments: " << std::endl << postProcIncr.inputIncrConfig_ << std::endl;
+      oops::Log::info() << "soca increments: " << std::endl
+                        << postProcIncr.inputIncrConfig_ << std::endl;
 
       // Process list of increments
       int result = 0;

utils/soca/gdas_postprocincr.h

@@ -1,9 +1,10 @@
-#ifndef GDAS_POSTPROCINCR_H
-#define GDAS_POSTPROCINCR_H
+#pragma once
 
-#include <iostream>
 #include <filesystem>
 
+#include <iostream>
+#include <string>
+
 #include "eckit/config/LocalConfiguration.h"
 
 #include "atlas/field.h"
@@ -22,7 +23,7 @@
 namespace gdasapp {
 
 class PostProcIncr {
-public:
+ public:
   // Constructor
   PostProcIncr(const eckit::Configuration & fullConfig, const soca::Geometry& geom,
                const eckit::mpi::Comm & comm)
@@ -33,7 +34,7 @@ class PostProcIncr {
       xTraj_(getTraj(fullConfig, geom)),
       comm_(comm),
       ensSize_(1),
-      pattern_(){
+      pattern_() {
 
     oops::Log::info() << "Date: " << std::endl << dt_ << std::endl;
 
@@ -73,18 +74,18 @@ class PostProcIncr {
   }
 
   // Append layer thicknesses to increment
-  // TODO: There's got to be a better way to append a variable.
-  soca::Increment appendLayer(const int n){
+  // TODO(guillaume): There's got to be a better way to append a variable.
+  soca::Increment appendLayer(const int n) {
     oops::Log::info() << "==========================================" << std::endl;
     oops::Log::info() << "======  Append Layers" << std::endl;
 
     // initialize the soca increment
     soca::Increment socaIncr(geom_, socaIncrVar_, dt_);
-    eckit::LocalConfiguration memberConfig; //inputIncrConfig_);
+    eckit::LocalConfiguration memberConfig;  // inputIncrConfig_);
     memberConfig = inputIncrConfig_;
 
     // replace templated string if necessary
-    if (not pattern_.empty()) {
+    if (!pattern_.empty()) {
       util::seekAndReplace(memberConfig, pattern_, std::to_string(n));
       oops::Log::info() << "oooooooooooooooooooooooooooooooooooo" << memberConfig << std::endl;
     }
@@ -124,7 +125,7 @@ class PostProcIncr {
   // Set specified variables to 0
   soca::Increment setToZero(soca::Increment socaIncr) {
     oops::Log::info() << "==========================================" << std::endl;
-    if (not this->setToZero_) {
+    if (!this->setToZero_) {
       oops::Log::info() << "======      no variables to set to 0.0" << std::endl;
       return socaIncr;
     }
@@ -154,7 +155,7 @@ class PostProcIncr {
   // Apply linear variable changes
   soca::Increment applyLinVarChange(soca::Increment socaIncr) {
     oops::Log::info() << "==========================================" << std::endl;
-    if (not this->doLVC_) {
+    if (!this->doLVC_) {
       return socaIncr;
     }
     oops::Log::info() << "======      applying specified change of variables" << std::endl;
@@ -252,8 +253,8 @@ class PostProcIncr {
   // Utility functions
   // -----------------------------------------------------------------------------
   // Recreate the soca filename from the configuration
-  // TODO: Change this in soca?
-  // TODO: Hard-coded for ocean, implement for seaice as well
+  // TODO(guillaume): Change this in soca?
+  // TODO(guillaume): Hard-coded for ocean, implement for seaice as well
   std::string socaFname() {
     std::string datadir;
     outputIncrConfig_.get("datadir", datadir);
@@ -284,7 +285,7 @@ class PostProcIncr {
 }
 
 
-public:
+ public:
   util::DateTime dt_;                  // valid date of increment
   oops::Variables layerVar_;           // layer variable
   const soca::Increment Layers_;       // layer thicknesses
@@ -303,6 +304,4 @@ class PostProcIncr {
   int ensSize_;
   std::string pattern_;
 };
-} // namespace gdasapp
-
-#endif // GDAS_POSTPROCINCR_H
+}  // namespace gdasapp

utils/soca/gdas_socahybridweights.h

@@ -1,6 +1,10 @@
+#pragma once
+
 #include <netcdf>
-#include <iostream>
+
 #include <filesystem>
+#include <iostream>
+#include <string>
 
 #include "eckit/config/LocalConfiguration.h"
 
@@ -13,9 +17,9 @@
 #include "oops/util/Duration.h"
 #include "oops/util/Logger.h"
 
-#include "soca/State/State.h"
 #include "soca/Geometry/Geometry.h"
 #include "soca/Increment/Increment.h"
+#include "soca/State/State.h"
 
 namespace gdasapp {
 
@@ -26,7 +30,6 @@ namespace gdasapp {
     static const std::string classname() {return "gdasapp::SocaHybridWeights";}
 
     int execute(const eckit::Configuration & fullConfig, bool /*validate*/) const {
-
       /// Setup the soca geometry
       const eckit::LocalConfiguration geomConfig(fullConfig, "geometry");
       oops::Log::info() << "geometry: " << std::endl << geomConfig << std::endl;
@@ -44,7 +47,7 @@ namespace gdasapp {
       socaVars += socaOcnVars;
 
       /// Read the background
-      // TODO: Use the ice extent to set the weights ... no clue if this is
+      // TODO(guillaume): Use the ice extent to set the weights ... no clue if this is
       //       possible at this level
       soca::State socaBkg(geom, socaVars, dt);
       const eckit::LocalConfiguration socaBkgConfig(fullConfig, "background");
@@ -59,7 +62,7 @@ namespace gdasapp {
       oops::Log::info() << "wOcean: " << wOcean << std::endl;
 
       /// Create fields of weights for seaice
-      soca::Increment socaIceHW(geom, socaVars, dt); // ocean field is mandatory for writting
+      soca::Increment socaIceHW(geom, socaVars, dt);  // ocean field is mandatory for writting
       socaIceHW.ones();
       socaIceHW *= wIce;
       oops::Log::info() << "socaIceHW: " << std::endl << socaIceHW << std::endl;

utils/test/CMakeLists.txt

@@ -6,8 +6,18 @@ list( APPEND utils_test_input
 file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/testinput)
 CREATE_SYMLINK( ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR} ${utils_test_input} )
 
+# copy the cpp linter script
+execute_process( COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/cpplint.py ${CMAKE_BINARY_DIR}/bin/${PROJECT_NAME}_cpplint.py)
+
+# add linter for the utils
+ecbuild_add_test( TARGET test_gdasapp_util_coding_norms
+                  TYPE SCRIPT
+                  COMMAND ${CMAKE_BINARY_DIR}/bin/${PROJECT_NAME}_cpplint.py
+                  ARGS --quiet --recursive ${CMAKE_CURRENT_SOURCE_DIR}/../
+                  WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/bin )
+
 # Test example IODA utility that computes the mean of a variable
 ecbuild_add_test( TARGET  test_gdasapp_util_ioda_example
                   COMMAND ${CMAKE_BINARY_DIR}/bin/gdas_meanioda.x
                   ARGS    "testinput/gdas_meanioda.yaml"
-                  LIBS    gdas-utils)
+                  LIBS    gdas-utils)