Repository: ontodev/robot
Pull request #727 (2020-08-24 17:56:38)

Fix row number in errors table
`validate` errors table row number is one off. This PR fixes that so that the cell reported is the correct cell.

- [x] `docs/` have been added/updated
- [x] tests have been added/updated
- [x] `mvn verify` says all tests pass
- [ ] `mvn site` says all JavaDocs correct
- [x] `CHANGELOG.md` has been updated



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 03ed2b97..7cb9611e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Handle empty [`template`] property charactersitics in [#719]
 - Fix referencing properties by CURIE in [`export`] in [#722]
 - Fix [`validate`] `--write-all true` in [#726]
+- Fix reported row number in [`validate`] error tables in [#727]
 
 ## [1.7.0] - 2020-07-31
 
@@ -201,6 +202,7 @@ First official release of ROBOT!
 [`template`]: http://robot.obolibrary.org/template
 [`validate`]: http://robot.obolibrary.org/validate
 
+[#726]: https://github.com/ontodev/robot/pull/727
 [#726]: https://github.com/ontodev/robot/pull/726
 [#722]: https://github.com/ontodev/robot/pull/722
 [#719]: https://github.com/ontodev/robot/pull/716
diff --git a/docs/examples/errors.csv b/docs/examples/errors.csv
new file mode 100644
index 00000000..021a3df7
--- /dev/null
+++ b/docs/examples/errors.csv
@@ -0,0 +1,15 @@
+ID,table,cell,level,rule ID,rule name,value,fix
+1,immune_exposures.csv,D8,error,immune_exposures!D3,"Cell is empty but rule: ""is-required true"" does not allow this.",,
+2,immune_exposures.csv,F9,error,immune_exposures!F3,"Cell is empty but rule: ""is-required true"" does not allow this.",,
+3,immune_exposures.csv,B10,error,immune_exposures!B3,"Validation failed for rule: ""Dengue virus equivalent-to (NCBITaxon:11060)"".",Dengue virus,
+4,immune_exposures.csv,C10,error,immune_exposures!C3,"Validation failed for rule: ""NCBITaxon:11060 equivalent-to 'Dengue virus'"".",NCBITaxon:11060,
+5,immune_exposures.csv,D11,error,immune_exposures!D3,"Validation failed for rule: ""Dengue virus 2 subclass-of disease"".",Dengue virus 2,
+6,immune_exposures.csv,D11,error,immune_exposures!D3,"Validation failed for rule: ""Dengue virus 2 subclass-of 'has material basis in' some 'Dengue virus'"".",Dengue virus 2,
+7,immune_exposures.csv,B12,error,immune_exposures!B3,"Validation failed for rule: ""blood or 'Hepacivirus C' equivalent-to (UBERON:0000178)"".",blood or 'Hepacivirus C',
+8,immune_exposures.csv,C12,error,immune_exposures!C3,"Validation failed for rule: ""UBERON:0000178 equivalent-to (blood or 'Hepacivirus C')"".",UBERON:0000178,
+9,immune_exposures.csv,F12,error,immune_exposures!F3,"Cell is non-empty (""Chronic"") but rule: ""is-excluded true"" does not allow this.",Chronic,
+10,immune_exposures.csv,D13,error,immune_exposures!D3,"Validation failed for rule: ""wheat allergy equivalent-to (DOID:12206)"".",wheat allergy,
+11,immune_exposures.csv,D13,error,immune_exposures!D3,"Validation failed for rule: ""dengue hemorrhagic fever equivalent-to (DOID:3660)"".",dengue hemorrhagic fever,
+12,immune_exposures.csv,D13,error,immune_exposures!D3,"Validation failed for rule: ""wheat allergy subclass-of 'has material basis in' some 'Dengue virus'"".",wheat allergy,
+13,immune_exposures.csv,E13,error,immune_exposures!E3,"Validation failed for rule: ""DOID:3660 equivalent-to 'dengue hemorrhagic fever'"".",DOID:3660,
+14,immune_exposures.csv,E13,error,immune_exposures!E3,"Validation failed for rule: ""DOID:12206 equivalent-to 'wheat allergy'"".",DOID:12206,
diff --git a/docs/examples/immune_exposures.txt b/docs/examples/immune_exposures.txt
index 562e98a5..5232c3f9 100644
--- a/docs/examples/immune_exposures.txt
+++ b/docs/examples/immune_exposures.txt
@@ -1,14 +1,14 @@
-At immune_exposures.csv row 6, column 4: Cell is empty but rule: "is-required true" does not allow this.
-At immune_exposures.csv row 7, column 6: Cell is empty but rule: "is-required true" does not allow this.
-At immune_exposures.csv row 8, column 2: Validation failed for rule: "Dengue virus equivalent-to (NCBITaxon:11060)".
-At immune_exposures.csv row 8, column 3: Validation failed for rule: "NCBITaxon:11060 equivalent-to 'Dengue virus'".
-At immune_exposures.csv row 9, column 4: Validation failed for rule: "Dengue virus 2 subclass-of disease".
-At immune_exposures.csv row 9, column 4: Validation failed for rule: "Dengue virus 2 subclass-of 'has material basis in' some 'Dengue virus'".
-At immune_exposures.csv row 10, column 2: Validation failed for rule: "blood or 'Hepacivirus C' equivalent-to (UBERON:0000178)".
-At immune_exposures.csv row 10, column 3: Validation failed for rule: "UBERON:0000178 equivalent-to (blood or 'Hepacivirus C')".
-At immune_exposures.csv row 10, column 6: Cell is non-empty ("Chronic") but rule: "is-excluded true" does not allow this.
-At immune_exposures.csv row 11, column 4: Validation failed for rule: "wheat allergy equivalent-to (DOID:12206)".
-At immune_exposures.csv row 11, column 4: Validation failed for rule: "dengue hemorrhagic fever equivalent-to (DOID:3660)".
-At immune_exposures.csv row 11, column 4: Validation failed for rule: "wheat allergy subclass-of 'has material basis in' some 'Dengue virus'".
-At immune_exposures.csv row 11, column 5: Validation failed for rule: "DOID:3660 equivalent-to 'dengue hemorrhagic fever'".
-At immune_exposures.csv row 11, column 5: Validation failed for rule: "DOID:12206 equivalent-to 'wheat allergy'".
+At immune_exposures.csv row 8, column 4: Cell is empty but rule: "is-required true" does not allow this.
+At immune_exposures.csv row 9, column 6: Cell is empty but rule: "is-required true" does not allow this.
+At immune_exposures.csv row 10, column 2: Validation failed for rule: "Dengue virus equivalent-to (NCBITaxon:11060)".
+At immune_exposures.csv row 10, column 3: Validation failed for rule: "NCBITaxon:11060 equivalent-to 'Dengue virus'".
+At immune_exposures.csv row 11, column 4: Validation failed for rule: "Dengue virus 2 subclass-of disease".
+At immune_exposures.csv row 11, column 4: Validation failed for rule: "Dengue virus 2 subclass-of 'has material basis in' some 'Dengue virus'".
+At immune_exposures.csv row 12, column 2: Validation failed for rule: "blood or 'Hepacivirus C' equivalent-to (UBERON:0000178)".
+At immune_exposures.csv row 12, column 3: Validation failed for rule: "UBERON:0000178 equivalent-to (blood or 'Hepacivirus C')".
+At immune_exposures.csv row 12, column 6: Cell is non-empty ("Chronic") but rule: "is-excluded true" does not allow this.
+At immune_exposures.csv row 13, column 4: Validation failed for rule: "wheat allergy equivalent-to (DOID:12206)".
+At immune_exposures.csv row 13, column 4: Validation failed for rule: "dengue hemorrhagic fever equivalent-to (DOID:3660)".
+At immune_exposures.csv row 13, column 4: Validation failed for rule: "wheat allergy subclass-of 'has material basis in' some 'Dengue virus'".
+At immune_exposures.csv row 13, column 5: Validation failed for rule: "DOID:3660 equivalent-to 'dengue hemorrhagic fever'".
+At immune_exposures.csv row 13, column 5: Validation failed for rule: "DOID:12206 equivalent-to 'wheat allergy'".
diff --git a/docs/validate.md b/docs/validate.md
index f2a6b927..a769856e 100644
--- a/docs/validate.md
+++ b/docs/validate.md
@@ -71,7 +71,15 @@ Note that the tooltips and styling will not work until the table is inserted int
 
 #### Errors Table (`--errors`)
 
-The results that are written to the output directory contain all lines from the input tables, even if those lines don't have errors. You can choose to also output a table containing just the errors by specifying `--errors <path>`. If this path ends with `.csv`, the output will be comma-separated. Otherwise, the output will be tab-separated.
+The results that are written to the output directory contain all lines from the input tables, even if those lines don't have errors. You can choose to also output a table containing just the errors by specifying `--errors <path>`:
+
+    robot validate --input immune_exposures.owl \
+      --table immune_exposures.csv \
+      --reasoner hermit \
+      --no-fail true \
+      --errors errors.csv
+
+If this path ends with `.csv`, the output will be comma-separated. Otherwise, the output will be tab-separated.
 
 This output will contain the following columns:
 * **ID**: an ID for the error - this starts at 1 and increases with each error
diff --git a/robot-core/src/main/java/org/obolibrary/robot/TableValidator.java b/robot-core/src/main/java/org/obolibrary/robot/TableValidator.java
index 7a4a6081..935c88ed 100644
--- a/robot-core/src/main/java/org/obolibrary/robot/TableValidator.java
+++ b/robot-core/src/main/java/org/obolibrary/robot/TableValidator.java
@@ -123,6 +123,7 @@ public class TableValidator {
   private Table outTable = null;
   private String currentTable;
   private int colNum;
+  private int rowIdx;
   private int rowNum;
   private boolean valid;
   private boolean silent;
@@ -211,11 +212,24 @@ public class TableValidator {
       // Get the header and rules rows
       List<String> headerRow = data.remove(0);
       List<String> rulesRow = data.remove(0);
+
+      // Get correct index for the rule row based on if a row was skipped
       int ruleRowIdx = 2;
       if (skippedRow < 3) {
         ruleRowIdx = 3;
       }
 
+      // Get number to add to rowIdx to get true row number from input table
+      // This will be either 3 or 4 (skipped row in header)
+      // as rowIdx starts at 0 and does not include header and rule rows
+      int addToRow;
+      if (skippedRow > 0 && skippedRow <= 3) {
+        // Skipped row is in header, add 1 to our reporting
+        addToRow = 4;
+      } else {
+        addToRow = 3;
+      }
+
       // Add header and rules rows to Table object
       for (int i = 0; i < headerRow.size(); i++) {
         String rawRule = i < rulesRow.size() ? rulesRow.get(i) : "";
@@ -223,19 +237,19 @@ public class TableValidator {
         Column c = new Column(headerRow.get(i), parseRules(rawRule), rawRule, provider);
         outTable.addColumn(c);
       }
-
       List<Column> columns = outTable.getColumns();
 
       // Validate data row by row, column by column
-      int add = 0;
-      for (rowNum = 0; rowNum < data.size(); rowNum++) {
-        if (rowNum + 1 == skippedRow) {
-          // Add 1 to each row num we report on to compensate for skipped row
-          add = 1;
+      for (rowIdx = 0; rowIdx < data.size(); rowIdx++) {
+        rowNum = rowIdx + addToRow;
+        if (rowNum == skippedRow) {
+          // Skipped row occurs in the data
+          addToRow = 4;
         }
-        List<String> row = data.get(rowNum);
+
+        List<String> row = data.get(rowIdx);
         if (!hasContent(row)) {
-          logger.debug(String.format("Skipping empty row %d", rowNum + 1 + add));
+          logger.debug(String.format("Skipping empty row %d", rowNum));
           continue;
         }
 
@@ -280,7 +294,7 @@ public class TableValidator {
                         new String[] {
                           String.valueOf(errCount),
                           currentTable,
-                          IOHelper.cellToA1(rowNum + 3 + add, colNum + 1),
+                          IOHelper.cellToA1(rowNum, colNum + 1),
                           "error",
                           FilenameUtils.getBaseName(currentTable)
                               + "!"
@@ -617,7 +631,9 @@ public class TableValidator {
     // Get the class expression corresponding to the rule that has been passed:
     OWLClassExpression ruleCE = getClassExpression(rule);
     if (ruleCE == null) {
-      report(String.format("Unable to parse rule \"%s %s\".", unsplitQueryType, rule));
+      report(
+          String.format(
+              "Unable to parse rule \"%s %s\" at column %d.", unsplitQueryType, rule, colNum + 1));
       return false;
     }
 
@@ -649,7 +665,7 @@ public class TableValidator {
       // class expression and run a generalised query on it:
       OWLClassExpression subjectCE = getClassExpression(subject);
       if (subjectCE == null) {
-        logger.error(String.format("Unable to parse subject \"%s\".", subject));
+        logger.error(String.format("Unable to parse subject \"%s\" at row %d.", subject, rowNum));
         return false;
       }
 
@@ -943,8 +959,7 @@ public class TableValidator {
     valid = false;
 
     // Format the error message
-    String outStr =
-        String.format("At %s row %d, column %d: ", currentTable, rowNum + 1, colNum + 1);
+    String outStr = String.format("At %s row %d, column %d: ", currentTable, rowNum, colNum + 1);
     outStr += String.format(format, positionalArgs);
     if (!silent) {
       // Print error if not silent
@@ -1030,8 +1045,7 @@ public class TableValidator {
     ArrayList<String[]> whenClauses = new ArrayList<>();
     for (String whenClause : whenClauseStr.split("\\s*&\\s*")) {
       m =
-          Pattern.compile(
-                  "^([^\'\\s()]+|\'[^\'()]+\'|\\(.+?\\))" + "\\s+([a-z\\-|]+)" + "\\s+(.*)$")
+          Pattern.compile("^([^\'\\s()]+|\'[^\']+\'|\\(.+?\\))" + "\\s+([a-z\\-|]+)" + "\\s+(.*)$")
               .matcher(whenClause);
 
       if (!m.find()) {


Test patch:
diff --git a/robot-core/src/test/resources/immune_exposures-result.txt b/robot-core/src/test/resources/immune_exposures-result.txt
index ed7464f2..7f0eacb0 100644
--- a/robot-core/src/test/resources/immune_exposures-result.txt
+++ b/robot-core/src/test/resources/immune_exposures-result.txt
@@ -1,18 +1,18 @@
 Validating immune_exposures.csv ...
-At immune_exposures.csv row 17, column 2: Cell is empty but rule: "is-required true" does not allow this.
-At immune_exposures.csv row 17, column 4: Unable to parse rule "subclass-of hasMaterialBasisIn some (null)".
-At immune_exposures.csv row 17, column 4: Validation failed for rule: "dengue hemorrhagic fever subclass-of hasMaterialBasisIn some (null)".
-At immune_exposures.csv row 19, column 4: Cell is empty but rule: "is-required true" does not allow this.
-At immune_exposures.csv row 20, column 6: Cell is empty but rule: "is-required true" does not allow this.
-At immune_exposures.csv row 21, column 2: Validation failed for rule: "Dengue virus equivalent-to 'Dengue virus 2'".
-At immune_exposures.csv row 21, column 3: Validation failed for rule: "NCBITaxon_11060 equivalent-to 'Dengue virus'".
-At immune_exposures.csv row 22, column 4: Validation failed for rule: "Dengue virus 2 subclass-of 'disease name'".
-At immune_exposures.csv row 22, column 4: Validation failed for rule: "Dengue virus 2 subclass-of hasMaterialBasisIn some 'Dengue virus'".
-At immune_exposures.csv row 23, column 2: Validation failed for rule: "'Hepatovirus A' or blood equivalent-to 'blood'".
-At immune_exposures.csv row 23, column 3: Validation failed for rule: "UBERON_0000178 equivalent-to ('Hepatovirus A' or blood)".
-At immune_exposures.csv row 23, column 6: Cell is non-empty ("chronic") but rule: "is-excluded true" does not allow this.
-At immune_exposures.csv row 24, column 4: Validation failed for rule: "wheat allergy equivalent-to 'dengue hemorrhagic fever'".
-At immune_exposures.csv row 24, column 4: Validation failed for rule: "dengue hemorrhagic fever equivalent-to 'wheat allergy'".
-At immune_exposures.csv row 24, column 4: Validation failed for rule: "wheat allergy subclass-of hasMaterialBasisIn some 'Dengue virus'".
-At immune_exposures.csv row 24, column 5: Validation failed for rule: "DOID_3660 equivalent-to 'dengue hemorrhagic fever'".
-At immune_exposures.csv row 24, column 5: Validation failed for rule: "DOID_12206 equivalent-to 'wheat allergy'".
+At immune_exposures.csv row 19, column 2: Cell is empty but rule: "is-required true" does not allow this.
+At immune_exposures.csv row 19, column 4: Unable to parse rule "subclass-of hasMaterialBasisIn some (null)" at column 4.
+At immune_exposures.csv row 19, column 4: Validation failed for rule: "dengue hemorrhagic fever subclass-of hasMaterialBasisIn some (null)".
+At immune_exposures.csv row 21, column 4: Cell is empty but rule: "is-required true" does not allow this.
+At immune_exposures.csv row 22, column 6: Cell is empty but rule: "is-required true" does not allow this.
+At immune_exposures.csv row 23, column 2: Validation failed for rule: "Dengue virus equivalent-to 'Dengue virus 2'".
+At immune_exposures.csv row 23, column 3: Validation failed for rule: "NCBITaxon_11060 equivalent-to 'Dengue virus'".
+At immune_exposures.csv row 24, column 4: Validation failed for rule: "Dengue virus 2 subclass-of 'disease name'".
+At immune_exposures.csv row 24, column 4: Validation failed for rule: "Dengue virus 2 subclass-of hasMaterialBasisIn some 'Dengue virus'".
+At immune_exposures.csv row 25, column 2: Validation failed for rule: "'Hepatovirus A' or blood equivalent-to 'blood'".
+At immune_exposures.csv row 25, column 3: Validation failed for rule: "UBERON_0000178 equivalent-to ('Hepatovirus A' or blood)".
+At immune_exposures.csv row 25, column 6: Cell is non-empty ("chronic") but rule: "is-excluded true" does not allow this.
+At immune_exposures.csv row 26, column 4: Validation failed for rule: "wheat allergy equivalent-to 'dengue hemorrhagic fever'".
+At immune_exposures.csv row 26, column 4: Validation failed for rule: "dengue hemorrhagic fever equivalent-to 'wheat allergy'".
+At immune_exposures.csv row 26, column 4: Validation failed for rule: "wheat allergy subclass-of hasMaterialBasisIn some 'Dengue virus'".
+At immune_exposures.csv row 26, column 5: Validation failed for rule: "DOID_3660 equivalent-to 'dengue hemorrhagic fever'".
+At immune_exposures.csv row 26, column 5: Validation failed for rule: "DOID_12206 equivalent-to 'wheat allergy'".