Skip to content

ENA-6078: Fixing partiality #159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Rajkumar-D wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ENA-6078: Fixing partiality #159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb59150
ENA-6078: Fixing partiality
Rajkumar-D Mar 25, 2024
632f8b7
ENA-6078: Fixing partiality
Rajkumar-D Mar 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/main/java/uk/ac/ebi/embl/api/entry/location/CompoundLocation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ public abstract class CompoundLocation<E extends Location> extends AbstractLocat

private final List<E> locations;

private boolean leftPartial;
private boolean rightPartial;
private boolean fivePrime;
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to:

  • fivePrimePartial
  • threePrimePartial

private boolean threePrime;

protected CompoundLocation() {
this.locations = new ArrayList<E>();
Expand Down Expand Up @@ -54,20 +54,20 @@ public boolean removeLocation(E location) {
return this.locations.remove(location);
}

public boolean isLeftPartial() {
return leftPartial;
public boolean isFivePrime() {
return fivePrime;
}

public void setLeftPartial(boolean leftPartial) {
this.leftPartial = leftPartial;
public void setFivePrime(boolean fivePrime) {
this.fivePrime = fivePrime;
}

public boolean isRightPartial() {
return rightPartial;
public boolean isThreePrime() {
return threePrime;
}

public void setRightPartial(boolean rightPartial) {
this.rightPartial = rightPartial;
public void setThreePrime(boolean threePrime) {
this.threePrime = threePrime;
}

@Override
Expand Down
20 changes: 20 additions & 0 deletions src/main/java/uk/ac/ebi/embl/api/entry/location/Location.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ public abstract class Location extends AbstractLocation implements Serializable
private Long beginPosition;
private Long endPosition;

private boolean threePrime;
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have the partiality in one place only or they may have conflicting values. If we have getters and setters in both CompoundLocation and Location then CompoundLocation should call the Location getters and setters and not have partiality fields itself. In this case CompoundLocation partiality setting should fail if there are no Locations yet.


private boolean fivePrime;

protected Location() {}

protected Location(Long beginPosition, Long endPosition, boolean complement) {
Expand Down Expand Up @@ -81,6 +85,22 @@ public Integer getIntEndPosition() {
return endPosition.intValue();
}

public boolean isThreePrime() {
return threePrime;
}

public void setThreePrime(boolean threePrime) {
this.threePrime = threePrime;
}

public boolean isFivePrime() {
return fivePrime;
}

public void setFivePrime(boolean fivePrime) {
this.fivePrime = fivePrime;
}

// 6361..6539,6363..6649
public boolean overlaps(Location location) {
if (location.getBeginPosition() >= getBeginPosition()
Expand Down
16 changes: 8 additions & 8 deletions src/main/java/uk/ac/ebi/embl/api/translation/CdsTranslator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,17 +140,17 @@ public ExtendedResult<TranslationResult> translate(CdsFeature cds, Entry entry)

if (translationResult.isFixedLeftPartial()) {
if (!cds.getLocations().isComplement()) {
cds.getLocations().setLeftPartial(translator.isLeftPartial());
cds.getLocations().setFivePrime(translator.isLeftPartial());
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe. Best would be to review change Translation left and right partiality as well.

} else {
cds.getLocations().setRightPartial(translator.isLeftPartial());
cds.getLocations().setThreePrime(translator.isLeftPartial());
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe. Best would be to review change Translation left and right partiality as well.

}
}

if (translationResult.isFixedRightPartial()) {
if (!cds.getLocations().isComplement()) {
cds.getLocations().setRightPartial(translator.isRightPartial());
cds.getLocations().setThreePrime(translator.isRightPartial());
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe. Best would be to review change Translation left and right partiality as well.

} else {
cds.getLocations().setLeftPartial(translator.isRightPartial());
cds.getLocations().setFivePrime(translator.isRightPartial());
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe. Best would be to review change Translation left and right partiality as well.

}
}

Expand Down Expand Up @@ -439,11 +439,11 @@ public ValidationResult configureFromFeature(
// then the left and right partiality are reversed between CdsFeature
// and Translator.
if (compoundLocation.isComplement()) {
translator.setLeftPartial(compoundLocation.isRightPartial());
translator.setRightPartial(compoundLocation.isLeftPartial());
translator.setLeftPartial(compoundLocation.isThreePrime());
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe. Best would be to review change Translation left and right partiality as well.

translator.setRightPartial(compoundLocation.isFivePrime());
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe. Best would be to review change Translation left and right partiality as well.

} else {
translator.setLeftPartial(compoundLocation.isLeftPartial());
translator.setRightPartial(compoundLocation.isRightPartial());
translator.setLeftPartial(compoundLocation.isFivePrime());
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe. Best would be to review change Translation left and right partiality as well.

translator.setRightPartial(compoundLocation.isThreePrime());
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe. Best would be to review change Translation left and right partiality as well.

}

// Set a pseudo translation.
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/uk/ac/ebi/embl/api/validation/check/entry/PeptideFeatureCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ private void checkTranslations(
/** ...if not, is the cds 5' or 3' partial?... */

/** is 3' partial? */
if (cdsFeature.getLocations().isLeftPartial()) {
if (cdsFeature.getLocations().isFivePrime()) {
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe.


Integer startCodon = cdsFeature.getStartCodon();
/** ...if partial, is the start codon 2 or 3?... */
Expand All @@ -154,7 +154,7 @@ private void checkTranslations(
}

/** is 5' partial? */
if (cdsFeature.getLocations().isRightPartial()) {
if (cdsFeature.getLocations().isThreePrime()) {
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe.

/** does the peptide end location match the end of a partial cds? */
Location cdsEnd = getEndFromLocations(cdsFeature.getLocations());
Location peptideEnd = getEndFromLocations(peptideFeature.getLocations());
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/uk/ac/ebi/embl/api/validation/check/feature/FeatureKeyCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ && getEmblEntryValidationPlanProperty()
private void validateCdsFeature(CdsFeature cdsFeature) {
CompoundLocation<Location> locations = cdsFeature.getLocations();

if (locations.isLeftPartial()
if (locations.isFivePrime()
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe.

&& !SequenceEntryUtils.isQualifierAvailable("codon_start", cdsFeature)) {
reportWarning(cdsFeature.getOrigin(), CODON_START_QUALIFIER_MESSAGE);
}
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/uk/ac/ebi/embl/api/validation/helper/location/LocationStringParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,19 @@ public CompoundLocation<Location> getCompoundLocation(String locationString, boo
Location location = getLocation(element.get(i));
if (isLeftPartial()) {
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isLeftPartial() should be changed as well before reviewing this code block.

if (!location.isComplement() && i == 0) {
compoundLocation.setLeftPartial(true);
compoundLocation.setFivePrime(true);
} else if (location.isComplement() && i == elementCount - 1) {
compoundLocation.setRightPartial(true);
compoundLocation.setThreePrime(true);
} else {
// Invalid location.
if (!ignoreError) return null;
}
}
if (isRightPartial()) {
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isRightPartial() should be changed as well before reviewing this code block.

if (location.isComplement() && i == 0) {
compoundLocation.setLeftPartial(true);
compoundLocation.setFivePrime(true);
} else if (!location.isComplement() && i == elementCount - 1) {
compoundLocation.setRightPartial(true);
compoundLocation.setThreePrime(true);
} else {
// Invalid location.
if (!ignoreError) return null;
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/uk/ac/ebi/embl/api/validation/helper/location/LocationToStringCoverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ public static String renderCompoundLocation(CompoundLocation<Location> compoundL
}
}

boolean leftPartial = compoundLocation.isLeftPartial();
boolean rightPartial = compoundLocation.isRightPartial();
boolean leftPartial = compoundLocation.isFivePrime();
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe.

boolean rightPartial = compoundLocation.isThreePrime();
Copy link
Contributor

@raskoleinonen raskoleinonen Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially unsafe.


if (locations.size() == 1) {
renderLocation(block, locations.get(0), leftPartial, rightPartial);
Expand Down
Loading