Issue 550 - Show modified peptide forms in sequence view #572
Conversation
|
|
||
| public void setPeptideForm(String peptideForm) | ||
| { | ||
| _peptideForm = Objects.requireNonNullElse(peptideForm, "combined"); |
There was a problem hiding this comment.
More opportunities to use a new constant
| @@ -94,18 +96,21 @@ public static Collection<Peptide> getPeptidesForGroup(long peptideGroupId) | |||
| return new SqlSelector(TargetedMSManager.getSchema(), sql).getArrayList(Peptide.class); | |||
| } | |||
|
|
|||
| public static List<PeptideCharacteristic> getPeptideCharacteristic(long peptideGroupId) | |||
| public static List<PeptideCharacteristic> getCombinedPeptideCharacteristics(long peptideGroupId, @Nullable Long replicateId) | |||
| { | |||
| var sqlDialect = TargetedMSManager.getSqlDialect(); | |||
| var pgLog10Intensity = "LOG(" + sqlDialect.getNumericCast(new SQLFragment("MAX(X.Intensity)")).getSQL() + ")"; | |||
There was a problem hiding this comment.
Shouldn't we worry about 0 values on Postgres too?
| { | ||
| var sqlDialect = TargetedMSManager.getSqlDialect(); | ||
| var pgLog10Intensity = "LOG(" + sqlDialect.getNumericCast(new SQLFragment("MAX(X.Intensity)")).getSQL() + ")"; | ||
| var sqlServerLog10Intensity = "LOG10(MAX(X.Intensity))"; | ||
| var sqlServerLog10Intensity = "CASE WHEN MAX(X.Intensity) IS NOT NULL AND MAX(X.Intensity) != 0 THEN LOG10(MAX(X.Intensity)) ELSE 0 END "; | ||
| var pgConfidenceValueToRound = "-LOG(" + sqlDialect.getNumericCast(new SQLFragment("MAX(X.Confidence)")).getSQL() + ")"; |
There was a problem hiding this comment.
I imagine it would be possible to have zeros for confidence as well
| @@ -115,10 +120,68 @@ public static List<PeptideCharacteristic> getPeptideCharacteristic(long peptideG | |||
| sql.append(" ON gm.id = pep.Id"); | |||
| sql.append(" INNER JOIN ").append(TargetedMSManager.getTableInfoSampleFile(), "sf"); | |||
| sql.append(" ON sf.Id = pci.SampleFileId"); | |||
| if (!Objects.equals(replicateId, Long.valueOf(0))) | |||
There was a problem hiding this comment.
If replicateId is nullable, why aren't we checking for null instead? I'd generally prefer to use null as the special values instead of 0
There was a problem hiding this comment.
I am sending 0 for 'All' value in the replicates dropdown from the jsp but yes I should check for null as well.
| } | ||
| sql.append(" GROUP BY pep.Sequence,pci.SampleFileId, pep.StartIndex, pep.EndIndex ) X "); | ||
| sql.append(" GROUP BY X.Sequence, X.StartIndex, X.EndIndex "); | ||
| sql.add(peptideGroupId); |
There was a problem hiding this comment.
I find it easier and more maintainable to add the value closer to the line that includes the ? in the SQL. As in, shift this up to right after calling sql.append(" WHERE gm.PeptideGroupId=? ");
| public static List<PeptideCharacteristic> getModifiedPeptideCharacteristics(long peptideGroupId, @Nullable Long replicateId) | ||
| { | ||
| var sqlDialect = TargetedMSManager.getSqlDialect(); | ||
| var pgLog10Intensity = "LOG(" + sqlDialect.getNumericCast(new SQLFragment("MAX(X.Intensity)")).getSQL() + ")"; |
There was a problem hiding this comment.
Can we consolidate at least the PG/SQLServer special casing with the previous method's? Maybe factor it out to an appendLogExpressions() method that takes the SqlDialect and appends the right variants?
| sql.append(" ON gm.id = pep.Id"); | ||
| sql.append(" INNER JOIN ").append(TargetedMSManager.getTableInfoSampleFile(), "sf"); | ||
| sql.append(" ON sf.Id = pci.SampleFileId"); | ||
| if (!Objects.equals(replicateId, Long.valueOf(0))) |
There was a problem hiding this comment.
Same question on null vs 0 here.
Rationale
This PR contains necessary changes to show combined vs stacked views of peptides in sequence view.
Related Pull Requests
Changes