UIMA
  1. UIMA
  2. UIMA-2387

ResultingAnnotationName not optional in ConceptMapper

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.3.1Addons
    • Fix Version/s: None
    • Labels:
      None

      Description

      Contrary to the documentation, the ResultingAnnotationName is not optional in the ConceptMapper descriptor. In our use case we only want to write back information to the original token, without creating a new annotation. Instead of treating this as a documentation bug it is therefore better to fix the code.

      1. UIMA-2387.patch
        8 kB
        Jens Grivolla

        Activity

        Hide
        Jens Grivolla added a comment -

        I just saw that trunk has changed since my patch, doing the same upgrades to JCasAnnotator_ImplBase instead of TextAnnotator that I have since done independently. This should make integration of UIMA-2387 easier, since I can rebase it on top of the current trunk to get a working AE instead of having to do it in two steps.

        Show
        Jens Grivolla added a comment - I just saw that trunk has changed since my patch, doing the same upgrades to JCasAnnotator_ImplBase instead of TextAnnotator that I have since done independently. This should make integration of UIMA-2387 easier, since I can rebase it on top of the current trunk to get a working AE instead of having to do it in two steps.
        Hide
        Richard Eckart de Castilho added a comment -

        Ok, I understand.

        I tried checking out the add-ons to try out the patch, but apparently the add-ons have had little attention for quite some time. At least, checking them out in Eclipse gives me lots of errors and red projects. Doesn't look like this is going to be a walk in the park...

        Show
        Richard Eckart de Castilho added a comment - Ok, I understand. I tried checking out the add-ons to try out the patch, but apparently the add-ons have had little attention for quite some time. At least, checking them out in Eclipse gives me lots of errors and red projects. Doesn't look like this is going to be a walk in the park...
        Hide
        Jens Grivolla added a comment -

        Btw, the whole patch consists in checking if the "ResultingAnnotationName" is defined and otherwise skipping the creation of a new annotation. That's why it just moves large chunks of code into an "if" block.

        At least for makeAnnotation() I could make the diff much smaller by putting "if (resultAnnotationType == null) return;" at the beginning instead of moving the whole method into a block. I don't know if that would be the preferred style (it would be more easily visible that the whole method gets skipped).

        Show
        Jens Grivolla added a comment - Btw, the whole patch consists in checking if the "ResultingAnnotationName" is defined and otherwise skipping the creation of a new annotation. That's why it just moves large chunks of code into an "if" block. At least for makeAnnotation() I could make the diff much smaller by putting "if (resultAnnotationType == null) return;" at the beginning instead of moving the whole method into a block. I don't know if that would be the preferred style (it would be more easily visible that the whole method gets skipped).
        Hide
        Jens Grivolla added a comment -

        I agree that the diff is not very pleasant to read, but it seems that the whitespace differences are due to chunks of code getting moved into an "if" block and thus increasing the indentation level. Other than some deleted blank lines it seems to me that all lines that appear in the diff are legitimately affected by the change. It's true that there is some inconsistency in indentation (2 vs. 4 spaces), but no reformatting of the existing code as far as I can see.

        How can I get it to be more readable?

        Show
        Jens Grivolla added a comment - I agree that the diff is not very pleasant to read, but it seems that the whitespace differences are due to chunks of code getting moved into an "if" block and thus increasing the indentation level. Other than some deleted blank lines it seems to me that all lines that appear in the diff are legitimately affected by the change. It's true that there is some inconsistency in indentation (2 vs. 4 spaces), but no reformatting of the existing code as far as I can see. How can I get it to be more readable?
        Hide
        Richard Eckart de Castilho added a comment -

        After a quick look at the diff, it appears that it is not minimal. There seem to be differences in the handling of whitespace in the diff. This might appear trivial, but it makes it take longer to see which lines are actually affected by the change. The patch also removes lines that are not relevant to the change.

        Can you provide a minimal patch against the current version?

        Show
        Richard Eckart de Castilho added a comment - After a quick look at the diff, it appears that it is not minimal. There seem to be differences in the handling of whitespace in the diff. This might appear trivial, but it makes it take longer to see which lines are actually affected by the change. The patch also removes lines that are not relevant to the change. Can you provide a minimal patch against the current version?
        Hide
        Jens Grivolla added a comment -

        Nobody interested in this? It's a simple fix for an annoying bug...

        Show
        Jens Grivolla added a comment - Nobody interested in this? It's a simple fix for an annoying bug...
        Hide
        Jens Grivolla added a comment -

        patch file created using git diff

        Show
        Jens Grivolla added a comment - patch file created using git diff

          People

          • Assignee:
            Jens Grivolla
            Reporter:
            Jens Grivolla
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development