#61: Apply TD reviews

DARMA-tasking · Sep 27, 2024 · 48f7aee · 48f7aee
1 parent 57bb67c
commit 48f7aee
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 35 deletions.
diff --git a/bindings/python/config_validator.cc b/bindings/python/config_validator.cc
@@ -10,8 +10,8 @@ namespace vt::tv::bindings::python {
   bool ConfigValidator::isValid()
   {
       bool is_valid = true;
-      for (std::string requiredParameter: required_parameters) {
-        if (!config[requiredParameter]) {
+      for (std::string parameter: required_parameters) {
+        if (!config[parameter]) {
           is_valid = false;
           break;
         }
@@ -27,16 +27,14 @@ namespace vt::tv::bindings::python {
    */
   std::string ConfigValidator::getMissingRequiredParameters()
   {
-    int i = 0;
     std::string parameters;
-    for (std::string requiredParameter: required_parameters) {
-      if (!config[requiredParameter]) {
-        if (i == 0 ) {
-          parameters = parameters + requiredParameter;
+    for (std::string parameter: required_parameters) {
+      if (!config[parameter]) {
+        if (parameters.empty()) {
+          parameters = parameter;
         } else {
-          parameters = parameters + ", " + requiredParameter;
+          parameters = parameters + ", " + parameter;
         }
-        i++;
       }
     }
     return parameters;

diff --git a/bindings/python/config_validator.h b/bindings/python/config_validator.h
@@ -40,7 +40,6 @@
 // *****************************************************************************
 //@HEADER
 */
-// A2DD.h
 #ifndef vt_tv_config_validator_h
 #define vt_tv_config_validator_h
 

diff --git a/bindings/python/tv.cc b/bindings/python/tv.cc
@@ -4,19 +4,25 @@
 namespace vt::tv::bindings::python {
 
 void tvFromJson(const std::vector<std::string>& input_json_per_rank_list, const std::string& input_yaml_params_str, uint64_t num_ranks) {
-  std::string startup_logo = std::string("        __           __\n")
-                           + std::string(" _   __/ /_         / /__   __\n")
-                           + std::string("| | / / __/ _____  / __/ | / /\n")
-                           + std::string("| |/ / /   /____/ / /_ | |/ /\n")
-                           + std::string("|___/\\__/         \\__/ |___/\n");
-  fmt::print("==============================\n");
-  fmt::print(startup_logo);
-  fmt::print("==============================\n");
-
-  // parse the input yaml parameters
-  try {
-    // Load the configuration from serialized YAML
-    YAML::Node viz_config = YAML::Load(input_yaml_params_str);
+    std::string startup_logo = std::string("        __           __\n")
+                             + std::string(" _   __/ /_         / /__   __\n")
+                             + std::string("| | / / __/ _____  / __/ | / /\n")
+                             + std::string("| |/ / /   /____/ / /_ | |/ /\n")
+                             + std::string("|___/\\__/         \\__/ |___/\n");
+    fmt::print("==============================\n");
+    fmt::print(startup_logo);
+    fmt::print("==============================\n");
+
+    YAML::Node viz_config;
+    try {
+      // Load the configuration from serialized YAML
+      viz_config = YAML::Load(input_yaml_params_str);
+    } catch (std::exception const& e) {
+        throw std::runtime_error(fmt::format(
+            "vt-tv: Error reading the configuration file: {}", 
+            e.what()
+        ));
+    }
 
     // Config Validator
     ConfigValidator config_validator(viz_config);
@@ -26,7 +32,10 @@ void tvFromJson(const std::vector<std::string>& input_json_per_rank_list, const
 
     // Throw error if configuration is invalid
     if (!is_config_valid) {
-      throw std::runtime_error("The YAML configuration file is not valid: missing required paramaters: " + config_validator.getMissingRequiredParameters());
+        throw std::runtime_error(fmt::format(
+            "vt-tv: Error validating the configuration file: {}", 
+            config_validator.getMissingRequiredParameters()
+        ));
     }
 
     std::array<std::string, 3> qoi_request = {
@@ -52,10 +61,16 @@ void tvFromJson(const std::vector<std::string>& input_json_per_rank_list, const
 
     // Throw an error if the output directory does not exist or is not absolute
     if (!std::filesystem::exists(output_path)) {
-      throw std::runtime_error(fmt::format("Visualization output directory does not exist at {}", output_dir));
+        throw std::runtime_error(fmt::format(
+            "vt-tv: Visualization output directory does not exist at {}", 
+            output_dir
+        ));
     }
     if (!output_path.is_absolute()) {
-      throw std::runtime_error("Visualization output directory must be absolute.");
+        throw std::runtime_error(fmt::format(
+            "vt-tv: Visualization output directory must be absolute: {}",
+            output_dir
+        ));
     }
 
     // append / to avoid problems with file stems
@@ -134,11 +149,8 @@ void tvFromJson(const std::vector<std::string>& input_json_per_rank_list, const
       output_dir, output_file_stem, 1.0, save_meshes, save_pngs, std::numeric_limits<PhaseType>::max()
     );
     render.generate(font_size, win_size);
-  } catch (std::exception const& e) {
-    throw std::runtime_error(fmt::format("vt-tv: Error reading the configuration file: {}", e.what()));
-  }
 
-  fmt::print("vt-tv: Done.\n");
+    fmt::print("vt-tv: Done.\n");
 }
 
 namespace nb = nanobind;

diff --git a/tests/test_bindings.py b/tests/test_bindings.py
@@ -9,15 +9,18 @@
 source_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 
 # Read the YAML config file
-with open(f'{source_dir}/tests/test_bindings_conf.yaml', 'r', encoding='utf-8') as stream:
+with open(f"{source_dir}/tests/test_bindings_conf.yaml", "r", encoding="utf-8") as stream:
     try:
         params = yaml.safe_load(stream)
     except yaml.YAMLError as exc:
         print(exc)
 
 # Check main key is "visualization"
 if "visualization" not in params:
-    print("The YAML configuration file is not valid: missing required paramaters: visualization")
+    print(
+        "The YAML configuration file is not valid: "+\
+        "missing required paramater \"visualization\""
+    )
     sys.exit(1)
 
 # make output_visualization_dir directory parameter absolute
@@ -29,16 +32,16 @@
 # Serialize visualization parameters
 params_serialized = yaml.dump(params["visualization"])
 
-# Calcul n_ranks
+# Calculate n_ranks
 n_ranks = params["visualization"]["x_ranks"] * \
     params["visualization"]["y_ranks"] * params["visualization"]["z_ranks"]
 
 rank_data = []
 for rank in range(n_ranks):
-    with open(f'{source_dir}/data/lb_test_data/data.{rank}.json', 'r', encoding='utf-8') as f:
+    with open(f"{source_dir}/data/lb_test_data/data.{rank}.json", "r", encoding="utf-8") as f:
         data = json.load(f)
 
-    # Add serialized  data into the rank
+    # Add serialized data into the rank
     rank_data.append((json.dumps(data)))
 
 # Launch VT TV from JSON data