Skip to content

Commit

Permalink
#4994 - Annotation browser sometimes does not load in sidebar curatio…
Browse files Browse the repository at this point in the history 
…n mode

- Make browser component more robust to even render if arc targets are missing (ignoring those arcs)
- Improve curation sidebar renderer to handle cases where a link source is hidden due to an already merged annotation
- Add hashCode and equals to Positions because it also implements compareTo
  • Loading branch information
@reckart
reckart committed Aug 20, 2024
1 parent 4e68a66 commit 2c10dc3
Show file tree
Hide file tree
Showing 16 changed files with 326 additions and 64 deletions.
6 changes: 2 additions & 4 deletions ...c/main/java/de/tudarmstadt/ukp/clarin/webanno/api/annotation/rendering/LabelRenderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,9 @@
import org.springframework.core.annotation.Order;

import de.tudarmstadt.ukp.clarin.webanno.api.annotation.config.AnnotationAutoConfiguration;
import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationLayer;
import de.tudarmstadt.ukp.inception.rendering.pipeline.RenderStep;
import de.tudarmstadt.ukp.inception.rendering.request.RenderRequest;
import de.tudarmstadt.ukp.inception.rendering.vmodel.VDocument;
import de.tudarmstadt.ukp.inception.rendering.vmodel.VObject;

/**
* <p>
Expand All @@ -49,8 +47,8 @@ public String getId()
@Override
public void render(VDocument aVDoc, RenderRequest aRequest)
{
for (AnnotationLayer layer : aVDoc.getAnnotationLayers()) {
for (VObject vobj : aVDoc.objects(layer.getId())) {
for (var layer : aVDoc.getAnnotationLayers()) {
for (var vobj : aVDoc.objects(layer.getId())) {
if (vobj.getLabelHint() != null) {
// Label hint was already set earlier - do not overwrite it!
continue;
Expand Down
11 changes: 11 additions & 0 deletions ...tation/src/main/java/de/tudarmstadt/ukp/inception/annotation/layer/span/SpanRenderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import static de.tudarmstadt.ukp.clarin.webanno.model.LinkMode.WITH_ROLE;
import static de.tudarmstadt.ukp.clarin.webanno.model.MultiValueMode.ARRAY;
import static de.tudarmstadt.ukp.inception.support.uima.ICasUtil.selectAnnotationByAddr;
import static de.tudarmstadt.ukp.inception.support.uima.ICasUtil.selectByAddr;
import static java.util.Collections.emptyList;
import static org.apache.commons.lang3.StringUtils.abbreviate;
Expand Down Expand Up @@ -123,6 +124,16 @@ public List<VLazyDetailGroup> lookupLazyDetails(CAS aCas, VID aVid)
group.addDetail(
new VLazyDetail("Text", abbreviate(fs.getCoveredText(), MAX_HOVER_TEXT_LENGTH)));

if (aVid.getAttribute() != VID.NONE) {
var adapter = getTypeAdapter();
var feature = adapter.listFeatures().stream().sequential().skip(aVid.getAttribute())
.findFirst().get();
List<LinkWithRoleModel> sourceLinks = adapter.getFeatureValue(feature, fs);
var target = selectAnnotationByAddr(aCas, sourceLinks.get(aVid.getSlot()).targetAddr);
group.addDetail(new VLazyDetail("Target",
abbreviate(target.getCoveredText(), MAX_HOVER_TEXT_LENGTH)));
}

var details = super.lookupLazyDetails(aCas, aVid);
if (!group.getDetails().isEmpty()) {
details.add(0, group);
Expand Down
39 changes: 18 additions & 21 deletions ...tion-legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/CasDiff.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,18 @@
import static java.util.Collections.emptyList;
import static java.util.Collections.emptySet;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toCollection;
import static java.util.stream.Stream.concat;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import java.util.stream.Stream;

import org.apache.commons.lang3.StringUtils;
import org.apache.uima.cas.ArrayFS;
Expand All @@ -42,7 +41,6 @@
import org.apache.uima.cas.FeatureStructure;
import org.apache.uima.cas.SofaFS;
import org.apache.uima.cas.Type;
import org.apache.uima.cas.TypeSystem;
import org.apache.uima.cas.text.AnnotationFS;
import org.apache.uima.fit.util.FSUtil;
import org.apache.uima.jcas.cas.AnnotationBase;
Expand Down Expand Up @@ -445,8 +443,8 @@ private boolean equalsFS(FeatureStructure aFS1, FeatureStructure aFS2)
return true;
}

Type type1 = aFS1.getType();
Type type2 = aFS2.getType();
var type1 = aFS1.getType();
var type2 = aFS2.getType();

// Types must be the same
if (!type1.getName().equals(type2.getName())) {
Expand All @@ -464,14 +462,13 @@ private boolean equalsFS(FeatureStructure aFS1, FeatureStructure aFS2)
// such as begin, end, etc. Mind that the types may come from different CASes at different
// levels of upgrading, so it could be that the types actually have slightly different
// features.
Set<String> labelFeatures = adapter.getLabelFeatures();
List<String> sortedFeatures = Stream
.concat(type1.getFeatures().stream().map(Feature::getShortName),
type2.getFeatures().stream().map(Feature::getShortName)) //
.filter(labelFeatures::contains) //
.sorted() //
.distinct() //
.collect(toList());
var labelFeatures = adapter.getLabelFeatures();
var sortedFeatures = concat(type1.getFeatures().stream().map(Feature::getShortName),
type2.getFeatures().stream().map(Feature::getShortName)) //
.filter(labelFeatures::contains) //
.sorted() //
.distinct() //
.collect(toCollection(ArrayList::new));

if (!recurseIntoLinkFeatures) {
// #1795 Chili REC: We can/should change CasDiff2 such that it does not recurse into
Expand Down Expand Up @@ -502,13 +499,13 @@ private boolean equalsFS(FeatureStructure aFS1, FeatureStructure aFS2)

switch (range.getName()) {
case CAS.TYPE_NAME_STRING_ARRAY: {
var value1 = FSUtil.getFeature(aFS1, f1, Set.class);
var value1 = f1 != null ? FSUtil.getFeature(aFS1, f1, Set.class) : null;
if (value1 == null) {
value1 = Collections.emptySet();
value1 = emptySet();
}
var value2 = FSUtil.getFeature(aFS2, f2, Set.class);
var value2 = f2 != null ? FSUtil.getFeature(aFS2, f2, Set.class) : null;
if (value2 == null) {
value2 = Collections.emptySet();
value2 = emptySet();
}
if (!value1.equals(value2)) {
return false;
Expand Down Expand Up @@ -589,8 +586,8 @@ private boolean equalsFS(FeatureStructure aFS1, FeatureStructure aFS2)
}
default: {
// Must be some kind of feature structure then
FeatureStructure valueFS1 = f1 != null ? aFS1.getFeatureValue(f1) : null;
FeatureStructure valueFS2 = f2 != null ? aFS2.getFeatureValue(f2) : null;
var valueFS1 = f1 != null ? aFS1.getFeatureValue(f1) : null;
var valueFS2 = f2 != null ? aFS2.getFeatureValue(f2) : null;

// Ignore the SofaFS - we already checked that the CAS is the same.
if (valueFS1 instanceof SofaFS) {
Expand All @@ -607,7 +604,7 @@ private boolean equalsFS(FeatureStructure aFS1, FeatureStructure aFS2)
// Q: Why do we not check recursively?
// A: Because e.g. for chains, this would mean we consider the whole chain as a
// single annotation, but we want to consider each link as an annotation
TypeSystem ts1 = aFS1.getCAS().getTypeSystem();
var ts1 = aFS1.getCAS().getTypeSystem();
if (ts1.subsumes(ts1.getType(CAS.TYPE_NAME_ANNOTATION), type1)) {
if (!equalsAnnotationFS((AnnotationFS) aFS1, (AnnotationFS) aFS2)) {
return false;
Expand Down
5 changes: 3 additions & 2 deletions ...ain/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/DiffAdapter_ImplBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
*/
package de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.api;

import static java.util.Collections.unmodifiableSet;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand All @@ -44,7 +45,7 @@ public abstract class DiffAdapter_ImplBase
public DiffAdapter_ImplBase(String aType, Set<String> aLabelFeatures)
{
type = aType;
labelFeatures = Collections.unmodifiableSet(new HashSet<>(aLabelFeatures));
labelFeatures = unmodifiableSet(new HashSet<>(aLabelFeatures));
}

public void addLinkFeature(String aName, String aRoleFeature, String aTargetFeature)
Expand Down
2 changes: 2 additions & 0 deletions ...legacy/src/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/Position.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,6 @@ public interface Position
String getDocumentId();

String toMinimalString();

boolean isLinkFeaturePosition();
}
95 changes: 91 additions & 4 deletions ...c/main/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/api/Position_ImplBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@
*/
package de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.api;

import java.util.Objects;

import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.builder.HashCodeBuilder;

import de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.LinkCompareBehavior;

Expand All @@ -30,16 +33,18 @@ public abstract class Position_ImplBase
private final String type;
private final String feature;

private final LinkCompareBehavior linkCompareBehavior;

private final String role;

private final int linkTargetBegin;
private final int linkTargetEnd;
private final String linkTargetText;

private final LinkCompareBehavior linkCompareBehavior;

// BEGIN: For debugging only - not included in compareTo/hashCode/equals!
private final String collectionId;
private final String documentId;
private final String linkTargetText;
// END: For debugging only - not included in compareTo/hashCode/equals!

public Position_ImplBase(String aCollectionId, String aDocumentId, String aType,
String aFeature, String aRole, int aLinkTargetBegin, int aLinkTargetEnd,
Expand All @@ -53,10 +58,10 @@ public Position_ImplBase(String aCollectionId, String aDocumentId, String aType,
role = aRole;
linkTargetBegin = aLinkTargetBegin;
linkTargetEnd = aLinkTargetEnd;
linkTargetText = aLinkTargetText;

collectionId = aCollectionId;
documentId = aDocumentId;
linkTargetText = aLinkTargetText;
}

@Override
Expand All @@ -77,6 +82,12 @@ public String getRole()
return role;
}

@Override
public boolean isLinkFeaturePosition()
{
return getFeature() != null;
}

@Override
public int getLinkTargetBegin()
{
Expand Down Expand Up @@ -115,6 +126,16 @@ public LinkCompareBehavior getLinkCompareBehavior()
@Override
public int compareTo(Position aOther)
{
// int collectionIdCmp = collectionId.compareTo(aOther.getCollectionId());
// if (collectionIdCmp != 0) {
// return collectionIdCmp;
// }
//
// int documentIdCmp = documentId.compareTo(aOther.getDocumentId());
// if (documentIdCmp != 0) {
// return documentIdCmp;
// }

int typeCmp = type.compareTo(aOther.getType());
if (typeCmp != 0) {
return typeCmp;
Expand Down Expand Up @@ -159,6 +180,72 @@ else if (linkCompareBehavior != null) {
}
}

@Override
public boolean equals(final Object other)
{
if (!(other instanceof Position_ImplBase)) {
return false;
}

Position_ImplBase castOther = (Position_ImplBase) other;
var result = // Objects.equals(collectionId, castOther.collectionId) //
// && Objects.equals(documentId, castOther.documentId) //
Objects.equals(type, castOther.type) //
&& Objects.equals(feature, castOther.feature) //
&& Objects.equals(linkCompareBehavior, castOther.linkCompareBehavior);

// If the base properties are equal, then we have to continue only linkCompareBehavior if it
// is non-null.
if (!result && linkCompareBehavior == null) {
return false;
}

switch (linkCompareBehavior) {
case LINK_TARGET_AS_LABEL:
// Include role into position
return Objects.equals(role, castOther.role);
case LINK_ROLE_AS_LABEL:
// Include target into position
return Objects.equals(linkTargetBegin, castOther.linkTargetBegin) //
&& Objects.equals(linkTargetEnd, castOther.linkTargetEnd);
default:
throw new IllegalStateException(
"Unknown link target comparison mode [" + linkCompareBehavior + "]");
}
}

@Override
public int hashCode()
{
var builder = new HashCodeBuilder() //
// .append(collectionId) //
// .append(documentId) //
.append(type) //
.append(feature) //
.append(linkCompareBehavior);

if (linkCompareBehavior == null) {
return builder.toHashCode();
}

switch (linkCompareBehavior) {
case LINK_TARGET_AS_LABEL:
// Include role into position
builder.append(role);
break;
case LINK_ROLE_AS_LABEL:
// Include target into position
builder.append(linkTargetBegin);
builder.append(linkTargetEnd);
break;
default:
throw new IllegalStateException(
"Unknown link target comparison mode [" + linkCompareBehavior + "]");
}

return builder.toHashCode();
}

protected void toStringFragment(StringBuilder builder)
{
if (getCollectionId() != null) {
Expand Down
24 changes: 24 additions & 0 deletions ...in/java/de/tudarmstadt/ukp/clarin/webanno/curation/casdiff/relation/RelationPosition.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
*/
package de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.relation;

import java.util.Objects;

import de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.LinkCompareBehavior;
import de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.api.Position;
import de.tudarmstadt.ukp.clarin.webanno.curation.casdiff.api.Position_ImplBase;
Expand Down Expand Up @@ -110,6 +112,28 @@ else if (targetBegin != otherSpan.targetBegin) {
}
}

@Override
public boolean equals(final Object other)
{
if (!(other instanceof RelationPosition)) {
return false;
}
if (!super.equals(other)) {
return false;
}
RelationPosition castOther = (RelationPosition) other;
return Objects.equals(sourceBegin, castOther.sourceBegin)
&& Objects.equals(sourceEnd, castOther.sourceEnd)
&& Objects.equals(targetBegin, castOther.targetBegin)
&& Objects.equals(targetEnd, castOther.targetEnd);
}

@Override
public int hashCode()
{
return Objects.hash(super.hashCode(), sourceBegin, sourceEnd, targetBegin, targetEnd);
}

@Override
public String toString()
{
Expand Down
Loading

0 comments on commit 2c10dc3

Please sign in to comment.