Update VariantDataServiceImpl to use new FrequencyData.Builder API to…

… merge and filter frequencyData
exomiser · Feb 7, 2024 · 176e5e6 · 176e5e6
1 parent ec3e7ca
commit 176e5e6
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 23 deletions.
diff --git a/...core/src/main/java/org/monarchinitiative/exomiser/core/genome/VariantDataServiceImpl.java b/...core/src/main/java/org/monarchinitiative/exomiser/core/genome/VariantDataServiceImpl.java
@@ -24,7 +24,6 @@
 import de.charite.compbio.jannovar.annotation.VariantEffect;
 import org.monarchinitiative.exomiser.core.genome.dao.*;
 import org.monarchinitiative.exomiser.core.model.Variant;
-import org.monarchinitiative.exomiser.core.model.frequency.Frequency;
 import org.monarchinitiative.exomiser.core.model.frequency.FrequencyData;
 import org.monarchinitiative.exomiser.core.model.frequency.FrequencySource;
 import org.monarchinitiative.exomiser.core.model.pathogenicity.ClinVarData;
@@ -97,24 +96,15 @@ public FrequencyData getVariantFrequencyData(Variant variant, Set<FrequencySourc
         // This could be run alongside the pathogenicities as they are all stored in the same datastore
         FrequencyData defaultFrequencyData = defaultFrequencyDao.getFrequencyData(variant);
 
-        List<Frequency> allFrequencies = filterFrequencies(variant, frequencySources, defaultFrequencyData);
-
-        return FrequencyData.of(defaultFrequencyData.getRsId(), allFrequencies);
-    }
-
-    private List<Frequency> filterFrequencies(Variant variant, Set<FrequencySource> frequencySources, FrequencyData defaultFrequencyData) {
-        List<Frequency> allFrequencies = new ArrayList<>();
-        for (Frequency frequency : defaultFrequencyData.getKnownFrequencies()) {
-            if (frequencySources.contains(frequency.getSource())) {
-                allFrequencies.add(frequency);
-            }
-        }
+        FrequencyData.Builder frequencyDataBuilder = defaultFrequencyData.toBuilder();
+        frequencyDataBuilder.filterSources(frequencySources);
 
         if (frequencySources.contains(FrequencySource.LOCAL)) {
             FrequencyData localFrequencyData = localFrequencyDao.getFrequencyData(variant);
-            allFrequencies.addAll(localFrequencyData.getKnownFrequencies());
+            frequencyDataBuilder.mergeFrequencyData(localFrequencyData);
         }
-        return allFrequencies;
+
+        return frequencyDataBuilder.build();
     }
 
     // PacBio data contains lots of longer non-symbolic variants with an SVTYPE
@@ -132,7 +122,6 @@ public PathogenicityData getVariantPathogenicityData(Variant variant, Set<Pathog
 
         ClinVarData clinVarData = clinVarDao.getClinVarData(variant);
 
-        // This could be run alongside the frequencies as they are all stored in the same datastore
         if (pathogenicitySources.isEmpty()) {
             // Fast-path for the unlikely case when no sources are defined - we'll just return the ClinVar data
             return PathogenicityData.of(clinVarData);

diff --git a/...core/src/main/java/org/monarchinitiative/exomiser/core/model/frequency/FrequencyData.java b/...core/src/main/java/org/monarchinitiative/exomiser/core/model/frequency/FrequencyData.java
@@ -445,6 +445,13 @@ public Builder rsId(String rsId) {
             return this;
         }
 
+        /**
+         *
+         * @param frequencySource
+         * @param frequency the frequency of the variant as a <b>percentage</b> value.
+         * @param homCount
+         * @return
+         */
         public Builder addFrequency(FrequencySource frequencySource, float frequency, int homCount) {
             frequencySources[frequencySource.ordinal()] = Objects.requireNonNull(frequencySource);
             frequencyData[frequencySource.ordinal() * WORD_SIZE + FREQ_OFFSET] = frequency;
@@ -453,20 +460,22 @@ public Builder addFrequency(FrequencySource frequencySource, float frequency, in
         }
 
         /**
-         * Removes any frequency data not in the input set of {@link FrequencySource}.
+         * Removes any frequency data not in the argument set of {@link FrequencySource}. Equivalent to a stream.filter()
+         * operation.
          *
          * @return the {@link FrequencyData.Builder} instance with potentially updated frequencies
          * @since 14.0.0
          */
-        public Builder retainSources(Set<FrequencySource> sourcesToRetain) {
+        public Builder filterSources(Set<FrequencySource> sourcesToRetain) {
             Objects.requireNonNull(sourcesToRetain);
             if (sourcesToRetain.isEmpty()) {
                 Arrays.fill(frequencySources, null);
                 // no need to set frequencyData elements to zero as these will be ignored when build() is called
                 return this;
             }
             for (int i = 0; i < frequencySources.length; i++) {
-                if (frequencySources[i] != null && !sourcesToRetain.contains(frequencySources[i])) {
+                FrequencySource frequencySource = frequencySources[i];
+                if (frequencySource != null && !sourcesToRetain.contains(frequencySource)) {
                     frequencySources[i] = null;
                     // no need to set frequencyData elements to zero as these will be ignored when build() is called
                 }

diff --git a/.../src/test/java/org/monarchinitiative/exomiser/core/model/frequency/FrequencyDataTest.java b/.../src/test/java/org/monarchinitiative/exomiser/core/model/frequency/FrequencyDataTest.java
@@ -285,7 +285,6 @@ public void testGetMaxFrequencyWithData() {
         Frequency minFrequency = Frequency.of(GNOMAD_G_AFR, 0.0002f);
         Frequency midFrequency = Frequency.of(GNOMAD_E_AMR, 25.5f);
         FrequencyData instance = FrequencyData.of(RS_ID, minFrequency, maxFrequency, midFrequency);
-        System.out.println(instance.getMaxFrequency());
         assertThat(instance.getMaxFrequency(), equalTo(maxFrequency));
     }
 
@@ -372,7 +371,7 @@ void testBuilderRetainSourcesEmpty() {
         FrequencyData instance = FrequencyData.builder()
                 .addFrequency(GNOMAD_E_AFR, 0.0005f, 0)
                 .addFrequency(GNOMAD_E_AMR, 0.0002f, 0)
-                .retainSources(Set.of())
+                .filterSources(Set.of())
                 .build();
         assertThat(instance, equalTo(FrequencyData.empty()));
     }
@@ -382,7 +381,7 @@ void testBuilderRetainSources() {
         FrequencyData instance = FrequencyData.builder()
                 .addFrequency(GNOMAD_E_AFR, 0.0005f, 0)
                 .addFrequency(GNOMAD_E_AMR, 0.0002f, 0)
-                .retainSources(Set.of(GNOMAD_E_AMR))
+                .filterSources(Set.of(GNOMAD_E_AMR))
                 .build();
 
         FrequencyData expected = FrequencyData.builder()
@@ -396,7 +395,7 @@ void testBuilderRetainSources() {
     void testBuilderRetainSourcesAllRemoved() {
         FrequencyData instance = FrequencyData.builder()
                 .addFrequency(GNOMAD_E_AFR, 0.0005f, 0)
-                .retainSources(Set.of(GNOMAD_E_SAS))
+                .filterSources(Set.of(GNOMAD_E_SAS))
                 .build();
         assertThat(instance, equalTo(FrequencyData.empty()));
     }