Skip to content

BI-1683 - Missing female parent in biparental cross results in dropped male parent v0.7 #235

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

Merged
timparsons merged 3 commits into from
Jan 4, 2023
Merged

BI-1683 - Missing female parent in biparental cross results in dropped male parent v0.7 #235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e472acf
[BI-1683] Preventing male parent from being imported without a female…
timparsons Dec 22, 2022
c01acaa
[BI-1683] Fixing unit tests
timparsons Dec 22, 2022
46a82c2
[BI-1683] Fixing display of pedigree string with unknown female parent
timparsons Jan 3, 2023
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
6 changes: 3 additions & 3 deletions src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
JsonObject additionalInfo = germplasm.getAdditionalInfo();
if(additionalInfo == null) {
additionalInfo = new JsonObject();
germplasm.setAdditionalInfo(additionalInfo);
}
additionalInfo.addProperty(BrAPIAdditionalInfoFields.GERMPLASM_RAW_PEDIGREE, germplasm.getPedigree());

Expand All @@ -182,6 +183,8 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
stream().filter(ref -> ref.getReferenceSource().equals(referenceSource)).
map(ref -> ref.getReferenceID()).findFirst().orElse("");
additionalInfo.addProperty(BrAPIAdditionalInfoFields.GERMPLASM_FEMALE_PARENT_GID, femaleParentAccessionNumber);
} else if (germplasm.getAdditionalInfo().has("femaleParentUnknown") && germplasm.getAdditionalInfo().get("femaleParentUnknown").getAsBoolean()) {
namePedigreeString = "Unknown";
}
}
if (parents.size() == 2) {
Expand All @@ -196,9 +199,6 @@ private Map<String,BrAPIGermplasm> processGermplasmForDisplay(List<BrAPIGermplas
}
}
//Add Unknown germplasm for display
if (germplasm.getAdditionalInfo().has("femaleParentUnknown") && germplasm.getAdditionalInfo().get("femaleParentUnknown").getAsBoolean()) {
namePedigreeString = "Unknown";
}
if (germplasm.getAdditionalInfo().has("maleParentUnknown") && germplasm.getAdditionalInfo().get("maleParentUnknown").getAsBoolean()) {
namePedigreeString += "/Unknown";
}
Expand Down
28 changes: 22 additions & 6 deletions ...main/java/org/breedinginsight/brapps/importer/services/processors/GermplasmProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public class GermplasmProcessor implements Processor {
public static String duplicateEntryNoMsg = "Entry numbers must be unique. Duplicated entry numbers found: %s";
public static String circularDependency = "Circular dependency in the pedigree tree";
public static String listNameAlreadyExists = "Import group name already exists";
public static String missingFemaleParent = "Female parent is missing. If the female parent is unknown, specify GID or entry number 0 as the female parent";
public static Function<List<String>, String> arrayOfStringFormatter = (lst) -> {
List<String> lstCopy = new ArrayList<>(lst);
Collections.sort(lstCopy);
Expand Down Expand Up @@ -259,6 +260,8 @@ public Map<String, ImportPreviewStatistics> process(List<BrAPIImport> importRows
}
}

validatePedigree(germplasm, i+2, validationErrors);

// Assign the entry number
if (germplasm.getEntryNo() == null) {
germplasm.setEntryNo(Integer.toString(i + 1));
Expand Down Expand Up @@ -330,6 +333,19 @@ public Map<String, ImportPreviewStatistics> process(List<BrAPIImport> importRows

}

private void validatePedigree(Germplasm germplasm, Integer rowNumber, ValidationErrors validationErrors) {
String femaleParentEntryNo = germplasm.getFemaleParentEntryNo();
String maleParentEntryNo = germplasm.getMaleParentEntryNo();
String femaleParentGID = germplasm.getFemaleParentDBID();
String maleParentGID = germplasm.getMaleParentDBID();

if(StringUtils.isNotBlank(maleParentEntryNo) && StringUtils.isBlank(femaleParentEntryNo) && StringUtils.isBlank(femaleParentGID)) {
validationErrors.addError(rowNumber, new ValidationError("Male Parent Entry No", missingFemaleParent, HttpStatus.UNPROCESSABLE_ENTITY));
} else if(StringUtils.isNotBlank(maleParentGID) && StringUtils.isBlank(femaleParentEntryNo) && StringUtils.isBlank(femaleParentGID)) {
validationErrors.addError(rowNumber, new ValidationError("Male Parent GID", missingFemaleParent, HttpStatus.UNPROCESSABLE_ENTITY));
}
}

private void createPostOrder() {
// Construct a dependency tree for POSTing order
Set<String> created = existingGermplasms.stream().map(BrAPIGermplasm::getGermplasmName).collect(Collectors.toSet());
Expand Down Expand Up @@ -441,13 +457,13 @@ public void constructPedigreeString(List<BrAPIImport> importRows, Map<Integer, P
boolean maleParentUnknown = false;

boolean femaleParentFound = false;
String pedigreeString = null;
StringBuilder pedigreeString = new StringBuilder();
if (femaleParentDB != null) {
if (femaleParentDB.equals("0")) {
femaleParentUnknown = true;
} else if (germplasmByAccessionNumber.containsKey(femaleParentDB)) {
BrAPIGermplasm femaleParent = germplasmByAccessionNumber.get(femaleParentDB).getBrAPIObject();
pedigreeString = commit ? femaleParent.getGermplasmName() : femaleParent.getDefaultDisplayName();
pedigreeString.append(commit ? femaleParent.getGermplasmName() : femaleParent.getDefaultDisplayName());
femaleParentFound = true;
}
} else if (femaleParentFile != null) {
Expand All @@ -457,7 +473,7 @@ public void constructPedigreeString(List<BrAPIImport> importRows, Map<Integer, P
else if (germplasmIndexByEntryNo.containsKey(germplasm.getFemaleParentEntryNo())) {
Integer femaleParentInd = germplasmIndexByEntryNo.get(femaleParentFile);
BrAPIGermplasm femaleParent = mappedBrAPIImport.get(femaleParentInd).getGermplasm().getBrAPIObject();
pedigreeString = commit ? femaleParent.getGermplasmName() : femaleParent.getDefaultDisplayName();
pedigreeString.append(commit ? femaleParent.getGermplasmName() : femaleParent.getDefaultDisplayName());
femaleParentFound = true;
}
}
Expand All @@ -469,7 +485,7 @@ else if (germplasmIndexByEntryNo.containsKey(germplasm.getFemaleParentEntryNo())
}
if ((germplasmByAccessionNumber.containsKey(germplasm.getMaleParentDBID()))) {
BrAPIGermplasm maleParent = germplasmByAccessionNumber.get(maleParentDB).getBrAPIObject();
if (!femaleParentUnknown) pedigreeString += String.format("/%s", commit ? maleParent.getGermplasmName() : maleParent.getDefaultDisplayName());
pedigreeString.append(String.format("/%s", commit ? maleParent.getGermplasmName() : maleParent.getDefaultDisplayName()));
}
} else if (maleParentFile != null){
if (maleParentFile.equals("0")) {
Expand All @@ -478,11 +494,11 @@ else if (germplasmIndexByEntryNo.containsKey(germplasm.getFemaleParentEntryNo())
if (germplasmIndexByEntryNo.containsKey(germplasm.getMaleParentEntryNo())) {
Integer maleParentInd = germplasmIndexByEntryNo.get(maleParentFile);
BrAPIGermplasm maleParent = mappedBrAPIImport.get(maleParentInd).getGermplasm().getBrAPIObject();
if (!femaleParentUnknown) pedigreeString += String.format("/%s", commit ? maleParent.getGermplasmName() : maleParent.getDefaultDisplayName());
pedigreeString.append(String.format("/%s", commit ? maleParent.getGermplasmName() : maleParent.getDefaultDisplayName()));
}
}
}
mappedBrAPIImport.get(i).getGermplasm().getBrAPIObject().setPedigree(pedigreeString);
mappedBrAPIImport.get(i).getGermplasm().getBrAPIObject().setPedigree(pedigreeString.length() > 0 ? pedigreeString.toString() : null);
//Simpler to just always add boolean, but consider for logic that previous imported values won't have that additional info value
mappedBrAPIImport.get(i).getGermplasm().getBrAPIObject().putAdditionalInfoItem("femaleParentUnknown", femaleParentUnknown);
mappedBrAPIImport.get(i).getGermplasm().getBrAPIObject().putAdditionalInfoItem("maleParentUnknown", maleParentUnknown);
Expand Down
5 changes: 3 additions & 2 deletions src/test/java/org/breedinginsight/brapps/importer/GermplasmTemplateMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ public void duplicateNameMarksDuplicates() {

@Test
@SneakyThrows
public void NoFemaleParentBlankPedigreeStringSuccess() {
public void OnlyMaleParentPreviewSuccess() {
File file = new File("src/test/resources/files/germplasm_import/no_female_parent_blank_pedigree.csv");

Flowable<HttpResponse<String>> call = uploadDataFile(file, "NoFemaleParentList", null, true);
Expand All @@ -369,7 +369,8 @@ public void NoFemaleParentBlankPedigreeStringSuccess() {
JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray();
for (int i = 0; i < previewRows.size(); i++) {
JsonObject germplasm = previewRows.get(i).getAsJsonObject().getAsJsonObject("germplasm").getAsJsonObject("brAPIObject");
assertTrue(!germplasm.has("pedigree"), "Pedigree string is populated, but should be empty");
assertTrue(germplasm.has("pedigree"), "Pedigree string should be populated, but is empty");
assertTrue(germplasm.get("pedigree").getAsString().substring(1).length() > 0, "Male pedigree string should be populated, but is empty");
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/test/resources/files/germplasm_import/no_female_parent_blank_pedigree.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Name,Breeding Method,Source,Female Parent GID,Male Parent GID,Entry No,Female Parent Entry No,Male Parent Entry No,External UID,Synonyms
Germplasm 1,BCR,Wild,,2,,,,1234,
Germplasm 2,BCR,Wild,,3,,,,5678,
Germplasm 3,BCR,Wild,,3,,,,9123,
Germplasm 1,BCR,Wild,0,2,,,,1234,
Germplasm 2,BCR,Wild,0,3,,,,5678,
Germplasm 3,BCR,Wild,0,3,,,,9123,