Robot header #1154

pk-mitre · 2023-09-29T21:09:21Z

Resolves #1152

docs/ have been added/updated
tests have been added/updated
mvn verify says all tests pass
mvn site says all JavaDocs correct
CHANGELOG.md has been updated

Adds option to input template strings via an external file as described in Issue #1152

… template strings

jamesaoverton · 2023-10-02T13:09:31Z

My first thought when I see the details of this implementation is that the row numbers in our error messages will be off by one, which will be confusing.

… numbers

jamesaoverton · 2023-11-29T15:21:34Z

CHANGELOG.md

@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

 ## [Unreleased]

+### Added
+-- Added option to input template strings from external file [##1152]


##1152 should be just #1152

This link needs an expansion at the bottom of the file.

jamesaoverton · 2023-11-29T15:21:41Z

docs/examples/animals_template_combined.tsv

+CURIE	Label	Parent	Comment
+ID	LABEL	SC %	A rdfs:comment
+obo:0000001	animal		Any animal in the world.
+0000002	canine	animal	A member of the genus Canis.


Is this combined file supposed to include this error? The file doesn't seem to be used anywhere, so I would not include it.

To answer my own question: Yes, I think this file is included to compare row numbers for errors. So I suggest naming this file animals_template_error.tsv to match animals_ext_template_error.tsv.

jamesaoverton · 2023-11-29T15:22:38Z

robot-core/src/main/java/org/obolibrary/robot/TemplateOperation.java

@@ -158,7 +159,7 @@ public static OWLOntology template(
   * Given an OWLOntology, an IOHelper, a map of table names to table contents, and a map of
   * template options, use the tables as templates to generate a merged output ontology.
   *
-   * @param inputOntology OWLOntology to use to get existing entities
+   * @param inputOntology OWLOntology to use to get existing entitfies


Should be "entities"

jamesaoverton · 2023-11-29T15:23:07Z

robot-core/src/main/java/org/obolibrary/robot/TemplateOperation.java

@@ -487,6 +492,8 @@ public static OWLOntology template(
      int idColumn = idColumns.get(tableName);
      int labelColumn = labelColumns.get(tableName);
      List<List<String>> rows = tables.get(tableName);
+      // TODO The starting row has to be controlled by the approach used, ie, internal or external


Is this TODO now done? If so the comment should be removed.

jamesaoverton · 2023-11-29T15:33:34Z

@pk-mitre I apologize for taking so long to review this PR. This is a good feature, and we really do appreciate your contribution! The problem is simply that nobody on the core ROBOT team has dedicated hours for this work.

I made a few specific suggestions. If you have time to reply or make those changes, it would be appreciated. If you don't have time, let me know and I'll try to find time to finish the PR myself.

The only major question I have at this point is: What to do if the template and the ext-template do not match? Eventually someone will add new columns to one and forget to add them to the other, then file a bug report here.

I think that that best approach might be to check that the first row of the template and the ext-template are identical. What do you think?

Thanks again!

merge

pk-mitre · 2024-01-09T20:32:44Z

James:
I have addressed the code comments and updated the pull request code.
I will follow up on the mismatch issue between the template and the ext-template
Thanks

jamesaoverton · 2024-02-07T19:47:38Z

@pk-mitre Thank you for your continued work on this PR. I'm sorry that our schedules never seem to be in sync.

Do you still plan to follow up on the case of a mismatch between the template and the ext-template? Please let me know either way. Thanks again!

pk-mitre · 2024-02-07T21:21:54Z

@jamesaoverton Yes, sorry I was working on a few other things. I will be following up on this mismatch issue and check things in.
Thanks !

pk-mitre · 2024-03-06T20:15:41Z

@jamesaoverton Am working through any potential additional error handling. With reference to your suggestion about checking the first row of the template file against the external template,I assume you mean the column count ? Since the first row in the template file is the header information, so it wouldn't be identical with the externalized template strings even in a correct case. For example:

Template file:

CURIE Label Parent Comment
obo:0000001 animal Any animal in the world.
obo:0000002 canine animal A member of the genus Canis.
obo:0000003 feline animal A member of the genus Felis.

external template file:

ID LABEL SC % A rdfs:comment

jamesaoverton · 2024-03-07T15:23:22Z

@pk-mitre You're right, my request was confused.

I'm worried about users changing the column structure of the template without changing the structure of the external template, or the other way around. I'm also worried that it's hard to tell which ROBOT template strings apply to which columns in the external template file.

So now I suggest that the external template file has two rows: the first for header strings and the second for ROBOT template strings. ROBOT should then check that the first row of the two files is the same: the headers in the template file match the headers in the external template file. If the first rows of the two files are not exactly the same, ROBOT should quit with an error message.

Template file:

CURIE Label Parent Comment
obo:0000001 animal Any animal in the world.
obo:0000002 canine animal A member of the genus Canis.
obo:0000003 feline animal A member of the genus Felis.

external template file:

CURIE Label Parent Comment
ID LABEL SC % A rdfs:comment

jamesaoverton · 2024-03-07T15:24:36Z

docs/template.md

+ROBOT template data read from separate external file
+
+    robot template --template animals_ext_template.tsv \
+        --ext-template animals_template.tsv \


I think it's confusing that --template uses animals_ext_template.tsv while --ext-template uses animals_template.tsv. The names of these files should be swapped.

jamesaoverton · 2024-03-07T15:26:05Z

robot-command/src/main/java/org/obolibrary/robot/TemplateCommand.java

@@ -49,6 +53,7 @@ public TemplateCommand() {
        "A", "include-annotations", true, "if true, include ontology annotations from merge input");
    o.addOption("f", "force", true, "if true, do not exit on error");
    o.addOption("e", "errors", true, "write errors to this path (TSV or CSV)");
+    o.addOption("E", "ext-template", true, "external robot template data file");


I'd prefer to call this external-template without abbreviating the name of the option.

…late

pk-mitre · 2024-03-27T18:31:14Z

@jamesaoverton I have made the changes for the external template header error checking and also the file name changes to make it clearer as indicated in your comment on the naming.

jamesaoverton · 2024-03-27T18:52:00Z

Thanks @pk-mitre! I skimmed the changes and they look good. I have a big deadline tomorrow, but I'll plan to finish reviewing on Friday.

jamesaoverton · 2024-03-29T20:35:08Z

I'm going to merge this and make a new PR with a few changes.

Sorry that this one took so long. Thanks for hanging in there and getting it done!

dlutz2 · 2024-03-30T07:06:03Z

@jamesaoverton Thanks for the merge, appreciate the work and the time.

pk-mitre · 2024-03-31T23:37:46Z

@jamesaoverton Thanks ! Glad it all merged in smoothly.

pk-mitre · 2024-04-17T16:43:15Z

@jamesaoverton This one line has to be updated to take into account the changed flag name:
if (options.get("external-template") != null) { template.setRowNum(1); }
File: TemplateOperation.java

jamesaoverton · 2024-04-17T17:57:01Z

You're right. I fixed that in dc04a3c. I really wish we had a test to catch that, but I ran into a bunch of problems verifying this, and used up more time than I could afford.

pk-mitre · 2024-04-18T14:56:46Z

@jamesaoverton Thanks for all your help on this ticket !

pk-mitre and others added 4 commits September 27, 2023 09:31

Updates for external template information file for tsv input

6cc5d58

Merge branch 'ontodev:master' into robot-header

9ed1ca5

Renamed external template file input option

c251d60

Added information about issue ontodev#1152 for external file input of…

fa51df2

… template strings

pk-mitre and others added 3 commits October 27, 2023 16:17

ontodev#1152 Updates to adjust rowNum when ext-template option is used

b2bb4b2

PR#1152 Added ext test template file with error to show adjusted line…

a5f7581

… numbers

Merge branch 'master' into robot-header

e29282c

jamesaoverton reviewed Nov 29, 2023

View reviewed changes

pk-mitre and others added 2 commits January 9, 2024 15:17

PR#1152 Addressed code review

948de75

Merge pull request #2 from ontodev/master

d3dfd8e

merge

jamesaoverton reviewed Mar 7, 2024

View reviewed changes

ontodev#1154 Code review changes and error checking for external temp…

11d9aea

…late

jamesaoverton mentioned this pull request Mar 29, 2024

Robot Header Updated #1189

Closed

5 tasks

jamesaoverton merged commit 7173c55 into ontodev:master Mar 29, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Robot header #1154

Robot header #1154

pk-mitre commented Sep 29, 2023 •

edited by jamesaoverton

Loading

jamesaoverton commented Oct 2, 2023

jamesaoverton Nov 29, 2023

jamesaoverton Nov 29, 2023

jamesaoverton Nov 29, 2023

jamesaoverton Nov 29, 2023

jamesaoverton Nov 29, 2023

jamesaoverton commented Nov 29, 2023

pk-mitre commented Jan 9, 2024 •

edited

Loading

jamesaoverton commented Feb 7, 2024

pk-mitre commented Feb 7, 2024

pk-mitre commented Mar 6, 2024

jamesaoverton commented Mar 7, 2024 •

edited

Loading

jamesaoverton Mar 7, 2024

jamesaoverton Mar 7, 2024

pk-mitre commented Mar 27, 2024

jamesaoverton commented Mar 27, 2024

jamesaoverton commented Mar 29, 2024

dlutz2 commented Mar 30, 2024

pk-mitre commented Mar 31, 2024

pk-mitre commented Apr 17, 2024

jamesaoverton commented Apr 17, 2024

pk-mitre commented Apr 18, 2024

Robot header #1154

Robot header #1154

Conversation

pk-mitre commented Sep 29, 2023 • edited by jamesaoverton Loading

jamesaoverton commented Oct 2, 2023

jamesaoverton Nov 29, 2023

Choose a reason for hiding this comment

jamesaoverton Nov 29, 2023

Choose a reason for hiding this comment

jamesaoverton Nov 29, 2023

Choose a reason for hiding this comment

jamesaoverton Nov 29, 2023

Choose a reason for hiding this comment

jamesaoverton Nov 29, 2023

Choose a reason for hiding this comment

jamesaoverton commented Nov 29, 2023

pk-mitre commented Jan 9, 2024 • edited Loading

jamesaoverton commented Feb 7, 2024

pk-mitre commented Feb 7, 2024

pk-mitre commented Mar 6, 2024

jamesaoverton commented Mar 7, 2024 • edited Loading

jamesaoverton Mar 7, 2024

Choose a reason for hiding this comment

jamesaoverton Mar 7, 2024

Choose a reason for hiding this comment

pk-mitre commented Mar 27, 2024

jamesaoverton commented Mar 27, 2024

jamesaoverton commented Mar 29, 2024

dlutz2 commented Mar 30, 2024

pk-mitre commented Mar 31, 2024

pk-mitre commented Apr 17, 2024

jamesaoverton commented Apr 17, 2024

pk-mitre commented Apr 18, 2024

pk-mitre commented Sep 29, 2023 •

edited by jamesaoverton

Loading

pk-mitre commented Jan 9, 2024 •

edited

Loading

jamesaoverton commented Mar 7, 2024 •

edited

Loading