Merge pull request #862 from finos/error-msg

Improved error messages from C++
finos · Jan 7, 2020 · ffd0ed2 · ffd0ed2
2 parents b13ac1e + a2d0967
commit ffd0ed2
Show file tree

Hide file tree

Showing 9 changed files with 32 additions and 21 deletions.
diff --git a/cpp/perspective/src/cpp/arrow_loader.cpp b/cpp/perspective/src/cpp/arrow_loader.cpp
@@ -73,8 +73,8 @@ namespace arrow {
                     std::shared_ptr<RecordBatch> chunk;
                     status = batch_reader->ReadRecordBatch(i, &chunk);
                     if (!status.ok()) {
-                        std::cerr << status.message() << std::endl;
-                        PSP_COMPLAIN_AND_ABORT(status.message());
+                        PSP_COMPLAIN_AND_ABORT(
+                            "Failed to read file record batch: " + status.message());
                     }
                     batches.push_back(chunk);
                 }
@@ -97,7 +97,7 @@ namespace arrow {
                 status = batch_reader->ReadAll(&m_table);
                 if (!status.ok()) {
                     std::stringstream ss;
-                    ss << "Failed to read batch: " << status.message() << std::endl;
+                    ss << "Failed to read stream record batch: " << status.message() << std::endl;
                     PSP_COMPLAIN_AND_ABORT(ss.str());
                 };
             }
@@ -333,7 +333,7 @@ namespace arrow {
                 for (uint32_t i = 0; i < len; ++i) {
                     ::arrow::Status status = vals[i].ToInteger(dest->get_nth<int64_t>(offset + i));
                     if (!status.ok()) {
-                        PSP_COMPLAIN_AND_ABORT(status.message());
+                        PSP_COMPLAIN_AND_ABORT("Could not write Decimal to column: " + status.message());
                     };
                 }
             } break;
@@ -347,7 +347,10 @@ namespace arrow {
                 }
             } break;
             default: {
-                PSP_COMPLAIN_AND_ABORT("Could not copy Arrow column of unknown type.");
+                std::stringstream ss;
+                std::string arrow_type = src->type()->ToString();
+                ss << "Could not load Arrow column of type `" << arrow_type << "`." << std::endl;
+                PSP_COMPLAIN_AND_ABORT(ss.str());
             }
         }
     }

diff --git a/cpp/perspective/src/cpp/arrow_writer.cpp b/cpp/perspective/src/cpp/arrow_writer.cpp
@@ -116,7 +116,7 @@ namespace arrow {
         std::shared_ptr<::arrow::Array> array;
         ::arrow::Status status = array_builder.Finish(&array);
         if (!status.ok()) {
-            PSP_COMPLAIN_AND_ABORT(status.message());
+            PSP_COMPLAIN_AND_ABORT("Could not serialize boolean column: " + status.message());
         }
         return array;
     }
@@ -158,7 +158,7 @@ namespace arrow {
         std::shared_ptr<::arrow::Array> array;
         ::arrow::Status status = array_builder.Finish(&array);
         if (!status.ok()) {
-            PSP_COMPLAIN_AND_ABORT(status.message());
+            PSP_COMPLAIN_AND_ABORT("Could not serialize date column: " + status.message());
         }
         return array;
     }
@@ -193,7 +193,7 @@ namespace arrow {
         std::shared_ptr<::arrow::Array> array;
         ::arrow::Status status = array_builder.Finish(&array);
         if (!status.ok()) {
-            PSP_COMPLAIN_AND_ABORT(status.message());
+            PSP_COMPLAIN_AND_ABORT("Could not serialize timestamp column: " + status.message());
         }
         return array;
     }

diff --git a/cpp/perspective/src/cpp/base.cpp b/cpp/perspective/src/cpp/base.cpp
@@ -292,8 +292,12 @@ str_to_dtype(const std::string& typestring) {
         return DTYPE_DATE;
     } else if (typestring == "datetime") {
         return DTYPE_TIME;
-    } else {
+    } else if (typestring == "string") {
         return DTYPE_STR;
+    } else {
+        PSP_COMPLAIN_AND_ABORT(
+            "Could not convert unknown type string `" + typestring + "` to dtype.");
+        return DTYPE_NONE;
     }
 }
 

diff --git a/cpp/perspective/src/cpp/schema.cpp b/cpp/perspective/src/cpp/schema.cpp
@@ -66,7 +66,8 @@ t_schema::get_colidx(const std::string& colname) const {
     auto iter = m_colidx_map.find(colname);
     if (iter == m_colidx_map.end()) {
         std::stringstream ss;
-        ss << "Column " << colname << " does not exist in schema." << std::endl;
+        ss << "Could not find column `" << colname 
+           << "` in the schema." << std::endl;
         PSP_COMPLAIN_AND_ABORT(ss.str());
     }
     return iter->second;
@@ -77,7 +78,8 @@ t_schema::get_dtype(const std::string& colname) const {
     auto iter = m_coldt_map.find(colname);
     if (iter == m_coldt_map.end()) {
         std::stringstream ss;
-        ss << "Column " << colname << " does not exist in schema." << std::endl;
+        ss << "Could not get type for column `" << colname 
+           << "` as it does not exist in the schema." << std::endl;
         PSP_COMPLAIN_AND_ABORT(ss.str());
     }
     return iter->second;
@@ -120,7 +122,10 @@ t_schema::retype_column(const std::string& colname, t_dtype dtype) {
         PSP_COMPLAIN_AND_ABORT("Cannot retype primary key or operation columns.");
     }
     if (!has_column(colname)) {
-        PSP_COMPLAIN_AND_ABORT("Cannot retype a column that does not exist.");
+        std::stringstream ss;
+        ss << "Cannot retype column `" << colname << "` as it does not exist."
+           << std::endl;
+        PSP_COMPLAIN_AND_ABORT(ss.str());
     }
 
     t_uindex idx = get_colidx(colname);

diff --git a/cpp/perspective/src/cpp/table.cpp b/cpp/perspective/src/cpp/table.cpp
@@ -180,8 +180,8 @@ Table::validate_columns(const std::vector<std::string>& column_names) {
         bool explicit_index
             = std::find(column_names.begin(), column_names.end(), m_index) != column_names.end();
         if (!explicit_index) {
-            std::cout << "Specified index " << m_index << " does not exist in data." << std::endl;
-            PSP_COMPLAIN_AND_ABORT("Specified index '" + m_index + "' does not exist in data.");
+            PSP_COMPLAIN_AND_ABORT(
+                "Specified index `" + m_index + "` does not exist in dataset.");
         }
     }
 }

diff --git a/cpp/perspective/src/cpp/view.cpp b/cpp/perspective/src/cpp/view.cpp
@@ -482,7 +482,6 @@ View<CTX_T>::data_slice_to_arrow(
     if (!valid.ok()) {
         std::stringstream ss;
         ss << "Invalid RecordBatch: " << valid.message() << std::endl;
-        std::cout << ss.str();
         PSP_COMPLAIN_AND_ABORT(ss.str());
     }
 

diff --git a/python/perspective/perspective/src/numpy.cpp b/python/perspective/perspective/src/numpy.cpp
@@ -469,7 +469,7 @@ namespace numpy {
                     return;
                 } break;
                 default:
-                    PSP_COMPLAIN_AND_ABORT("Unable to fill non-numeric column in `fill_numeric_iter`.")
+                    PSP_COMPLAIN_AND_ABORT("Unable to fill non-numeric column `"  + name + "` in `fill_numeric_iter`.")
             }
         }
 

diff --git a/python/perspective/perspective/tests/table/test_table_arrow.py b/python/perspective/perspective/tests/table/test_table_arrow.py
@@ -365,7 +365,7 @@ def test_table_arrow_loads_float_legacy(self, util):
             "b": data[1]
         }
 
-    def test_table_arrow_loads_decimal_legacy(self, util):
+    def test_table_arrow_loads_decimal128_legacy(self, util):
         data = [
             [i * 1000 for i in range(10)]
         ]

diff --git a/python/perspective/perspective/tests/table/test_update_arrow.py b/python/perspective/perspective/tests/table/test_update_arrow.py
@@ -124,13 +124,13 @@ def test_update_arrow_updates_float_stream(self, util):
             "b": data[1]
         }
 
-    def test_update_arrow_updates_decimal_stream(self, util):
+    def test_update_arrow_updates_decimal128_stream(self, util):
         data = [
-            [i * 1000 for i in range(10)]
+            [i * 1000000000 for i in range(10)]
         ]
-        arrow_data = util.make_arrow(["a"], data, types=[pa.decimal128(4)])
+        arrow_data = util.make_arrow(["a"], data, types=[pa.decimal128(10)])
         tbl = Table({
-            "a": int,
+            "a": int
         })
         tbl.update(arrow_data)
         assert tbl.size() == 10