Merge branch 'develop' into bugfix/intermittently_failing_tests

ebi-gene-expression-group · Nov 27, 2024 · 136f216 · 136f216
2 parents d3237ae + a89cd50
commit 136f216
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 102 deletions.
diff --git a/app/src/main/java/uk/ac/ebi/atlas/experimentpage/JsonMarkerGenesController.java b/app/src/main/java/uk/ac/ebi/atlas/experimentpage/JsonMarkerGenesController.java
@@ -30,27 +30,27 @@ public JsonMarkerGenesController(HighchartsHeatmapAdapter highchartsHeatmapAdapt
     public String getClusterMarkerGenes(@PathVariable String experimentAccession,
                                         @RequestParam String k) {
         return GSON.toJson(
-                highchartsHeatmapAdapter.getMarkerGeneHeatmapDataSortedNaturally(
-                        markerGeneService.getMarkerGenesPerCluster(experimentAccession, k)
-                ));
+            highchartsHeatmapAdapter.getSortedMarkerGeneData(
+                markerGeneService.getMarkerGenesPerCluster(experimentAccession, k)
+            ));
     }
 
     @GetMapping(value = "/json/experiments/{experimentAccession}/marker-genes/cell-types",
                 produces = MediaType.APPLICATION_JSON_UTF8_VALUE)
     public String getCellTypeMarkerGenes(@PathVariable String experimentAccession,
                                          @RequestParam Set<String> organismPart) {
-        return GSON.toJson(highchartsHeatmapAdapter.getMarkerGeneHeatmapDataSortedLexicographically(
-                markerGeneService.getCellTypeMarkerGeneProfile(experimentAccession, ImmutableSet.copyOf(organismPart))
+        return GSON.toJson(highchartsHeatmapAdapter.getSortedMarkerGeneData(
+            markerGeneService.getCellTypeMarkerGeneProfile(experimentAccession, ImmutableSet.copyOf(organismPart))
         ));
     }
 
     @GetMapping(value = "/json/experiments/{experimentAccession}/marker-genes-heatmap/cell-types",
                 produces = MediaType.APPLICATION_JSON_UTF8_VALUE)
     public String getCellTypeMarkerGenesHeatmapData(@PathVariable String experimentAccession,
-                                                @RequestParam String cellGroupType) {
-        return GSON.toJson(highchartsHeatmapAdapter.getMarkerGeneHeatmapDataSortedLexicographically(
+                                                    @RequestParam String cellGroupType) {
+        return GSON.toJson(
+            highchartsHeatmapAdapter.getSortedMarkerGeneData(
                 markerGeneService.getCellTypeMarkerGeneHeatmapData(experimentAccession, cellGroupType)
-        ));
+            ));
     }
-
 }
diff --git a/app/src/main/java/uk/ac/ebi/atlas/experimentpage/markergenes/ClusterNameComparator.java b/app/src/main/java/uk/ac/ebi/atlas/experimentpage/markergenes/ClusterNameComparator.java
@@ -0,0 +1,23 @@
+package uk.ac.ebi.atlas.experimentpage.markergenes;
+
+import java.text.Collator;
+import java.util.Comparator;
+import java.util.Locale;
+
+class ClusterNameComparator implements Comparator<String> {
+
+    @Override
+    public int compare(String o1, String o2) {
+        final String regexForNumeric = "^[0-9]*$";
+
+        if (o1.matches(regexForNumeric) && o2.matches(regexForNumeric)) {
+            return Integer.compare(Integer.parseInt(o1), Integer.parseInt(o2));
+        } else if (o1.matches(regexForNumeric)) {
+            return 1;
+        } else if (o2.matches(regexForNumeric)) {
+            return -1;
+        } else {
+            return Collator.getInstance(Locale.US).compare(o1, o2);
+        }
+    }
+}
diff --git a/app/src/main/java/uk/ac/ebi/atlas/experimentpage/markergenes/HighchartsHeatmapAdapter.java b/app/src/main/java/uk/ac/ebi/atlas/experimentpage/markergenes/HighchartsHeatmapAdapter.java
@@ -12,35 +12,27 @@
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static java.util.Comparator.comparing;
 import static java.util.stream.Collectors.groupingBy;
 
 @Component
 public class HighchartsHeatmapAdapter {
     private static final Function<MarkerGene, Pair<String, String>> MARKER_GENE_ID_TO_CELL_GROUP_VALUE_WHERE_MARKER =
             markerGene -> Pair.of(markerGene.geneId(), markerGene.cellGroupValueWhereMarker());
 
-    private static final Comparator<MarkerGene> CELL_GROUP_VALUE_WHERE_MARKER_LEXICOGRAPHICAL =
-            comparing(MarkerGene::cellGroupValueWhereMarker).thenComparing(MarkerGene::pValue);
-
     private static final Comparator<MarkerGene> MARKER_NATURALLY_ORDERED_BY_CELL_GROUP_VALUE =
-            new MarkerGeneComparatorByCellGroupValueByMarker();
+            new MarkerGeneComparatorByCellGroupValueWhereMarker();
 
     private static final Comparator<MarkerGene> MARKER_GENE_COMPARATOR =
             MARKER_NATURALLY_ORDERED_BY_CELL_GROUP_VALUE.thenComparing(MarkerGene::pValue);
 
+    private static final Comparator<String> CLUSTER_NAME_COMPARATOR = new ClusterNameComparator();
+
     private final BioEntityPropertyDao bioEntityPropertyDao;
 
     public HighchartsHeatmapAdapter(BioEntityPropertyDao bioEntityPropertyDao) {
         this.bioEntityPropertyDao = bioEntityPropertyDao;
     }
 
-    public ImmutableList<ImmutableMap<String, Object>> getMarkerGeneHeatmapDataSortedNaturally
-            (Collection<MarkerGene> markerGenes) {
-
-        return getMarkerGeneHeatmapData(markerGenes, MARKER_GENE_COMPARATOR);
-    }
-
     /**
      * Given a list of marker genes, this method returns a Highcharts data series object
      * (<a href="https://api.highcharts.com/highcharts/series.heatmap.data">heatmap data for series</a>), where gene IDs/symbols are
@@ -49,16 +41,21 @@ public HighchartsHeatmapAdapter(BioEntityPropertyDao bioEntityPropertyDao) {
      * The rows of the heatmap are ordered by the cell type, i.e. genes for celltype 1, 2, etc.
      * If there are no marker genes for a cell group, then no rows will be present in the data.
      */
-    public ImmutableList<ImmutableMap<String, Object>> getMarkerGeneHeatmapDataSortedLexicographically(
-            Collection<MarkerGene> markerGenes) {
+    public ImmutableList<ImmutableMap<String, Object>> getSortedMarkerGeneData
+            (Collection<MarkerGene> markerGenes) {
+        var sortedMarkerGenes = getSortedMarkerGenes(markerGenes);
+
+        var rows = getRowsFromSortedMarkerGenes(sortedMarkerGenes);
+
+        var columns = getColumnsFromSortedMarkerGenes(sortedMarkerGenes);
 
-        return getMarkerGeneHeatmapData(markerGenes, CELL_GROUP_VALUE_WHERE_MARKER_LEXICOGRAPHICAL);
+        return getMarkerGeneHeatmapData(sortedMarkerGenes, rows, columns);
     }
 
-    private ImmutableList<MarkerGene> getSortedMarkerGenes(Collection<MarkerGene> markerGenes, Comparator<MarkerGene> markerGeneComparator) {
+    private ImmutableList<MarkerGene> getSortedMarkerGenes(Collection<MarkerGene> markerGenes) {
         return mergeSameGeneIdIntoSingleGroup(markerGenes).stream()
                 .parallel()
-                .sorted(markerGeneComparator)
+                .sorted(MARKER_GENE_COMPARATOR)
                 .collect(toImmutableList());
     }
 
@@ -73,18 +70,13 @@ private static ImmutableList<String> getColumnsFromSortedMarkerGenes(ImmutableLi
         return sortedMarkerGenes.stream()
                 .map(MarkerGene::cellGroupValue)
                 .distinct()
-                .sorted()
+                .sorted(CLUSTER_NAME_COMPARATOR)
                 .collect(toImmutableList());
     }
 
-    private ImmutableList<ImmutableMap<String, Object>> getMarkerGeneHeatmapData(Collection<MarkerGene> markerGenes,
-                                                                                 Comparator<MarkerGene> markerGeneComparator) {
-        var sortedMarkerGenes = getSortedMarkerGenes(markerGenes, markerGeneComparator);
-
-        var rows = getRowsFromSortedMarkerGenes(sortedMarkerGenes);
-
-        var columns = getColumnsFromSortedMarkerGenes(sortedMarkerGenes);
-
+    private ImmutableList<ImmutableMap<String, Object>> getMarkerGeneHeatmapData(Collection<MarkerGene> sortedMarkerGenes,
+                                                                                 ImmutableList<Pair<String, String>> rows,
+                                                                                 ImmutableList<String> columns) {
         var symbolsForGeneIds =
                 bioEntityPropertyDao.getSymbolsForGeneIds(
                         sortedMarkerGenes.stream().map(MarkerGene::geneId).collect(toImmutableSet()));

diff --git a/...ac/ebi/atlas/experimentpage/markergenes/MarkerGeneComparatorByCellGroupValueByMarker.java b/...ac/ebi/atlas/experimentpage/markergenes/MarkerGeneComparatorByCellGroupValueByMarker.java
diff --git a/...ebi/atlas/experimentpage/markergenes/MarkerGeneComparatorByCellGroupValueWhereMarker.java b/...ebi/atlas/experimentpage/markergenes/MarkerGeneComparatorByCellGroupValueWhereMarker.java
@@ -0,0 +1,13 @@
+package uk.ac.ebi.atlas.experimentpage.markergenes;
+
+import java.util.Comparator;
+
+class MarkerGeneComparatorByCellGroupValueWhereMarker implements Comparator<MarkerGene> {
+
+    public int compare(MarkerGene marker1, MarkerGene marker2) {
+        var clusterNameComparator = new ClusterNameComparator();
+
+        return clusterNameComparator.compare(
+            marker1.cellGroupValueWhereMarker(),marker2.cellGroupValueWhereMarker());
+    }
+}
diff --git a/...ava/uk/ac/ebi/atlas/search/metadata/JsonMultiexperimentCellTypeMarkerGenesController.java b/...ava/uk/ac/ebi/atlas/search/metadata/JsonMultiexperimentCellTypeMarkerGenesController.java
@@ -1,7 +1,6 @@
 package uk.ac.ebi.atlas.search.metadata;
 
 import com.google.common.collect.ImmutableSet;
-import org.jetbrains.annotations.NotNull;
 import org.springframework.http.MediaType;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
@@ -10,8 +9,6 @@
 import uk.ac.ebi.atlas.controllers.JsonExceptionHandlingController;
 import uk.ac.ebi.atlas.experimentpage.markergenes.HighchartsHeatmapAdapter;
 
-import java.net.URLDecoder;
-import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.Base64;
 import java.util.Collection;
@@ -36,7 +33,7 @@ public String getCellTypeMarkerGenes(
             @PathVariable String cellType,
             @RequestParam(name = "experiment-accessions", required = false) Collection<String> experimentAccessions) {
         return GSON.toJson(
-                highchartsHeatmapAdapter.getMarkerGeneHeatmapDataSortedLexicographically(
+                highchartsHeatmapAdapter.getSortedMarkerGeneData(
                         experimentAccessions == null ?
                                 multiexperimentCellTypeMarkerGenesService.getCellTypeMarkerGeneProfile(
                                         getDecodedCellType(cellType)) :

diff --git a/...rc/test/java/uk/ac/ebi/atlas/experimentpage/markergenes/HighchartsHeatmapAdapterTest.java b/...rc/test/java/uk/ac/ebi/atlas/experimentpage/markergenes/HighchartsHeatmapAdapterTest.java
@@ -55,7 +55,7 @@ void markerGeneWithoutSymbolHasGeneIDAsName() {
                 MarkerGene.create(randomGeneIds.get(2), "1", "5", 0.001,
                         "6", 1000, 10000, EXPRESSION_UNIT));
 
-        var result = subject.getMarkerGeneHeatmapDataSortedNaturally(markerGenes);
+        var result = subject.getSortedMarkerGeneData(markerGenes);
         assertThat(result).hasSize(3);
 
         assertThat(result).element(0).extracting("geneName").containsOnly(randomGeneSymbols.get(0));
@@ -85,7 +85,7 @@ void cellTypeMarkerGeneWithoutSymbolHasGeneIDAsName() {
                         "T cell", 0.001,
                         "B cell", 1000, 10000, EXPRESSION_UNIT));
 
-        var result = subject.getMarkerGeneHeatmapDataSortedLexicographically(cellTypeMarkerGenes);
+        var result = subject.getSortedMarkerGeneData(cellTypeMarkerGenes);
 
         assertThat(result).hasSize(3);
         assertThat(result).element(0).extracting("geneName").containsOnly(randomGeneSymbols.get(0));
@@ -131,15 +131,15 @@ void mergesMultipleGeneIdsAcrossGroupsIntoOneRowWithLowestPValue() {
         when(bioEntityPropertyDaoMock.getSymbolsForGeneIds(ImmutableSet.of(geneId)))
                 .thenReturn(ImmutableMap.of(geneId, geneId));
 
-        assertThat(subject.getMarkerGeneHeatmapDataSortedLexicographically(ImmutableSet.of(markerGene1, markerGene2)))
+        assertThat(subject.getSortedMarkerGeneData(ImmutableSet.of(markerGene1, markerGene2)))
                 .extracting("y")
                 .containsOnly(0);
 
-        assertThat(subject.getMarkerGeneHeatmapDataSortedNaturally(ImmutableSet.of(markerGene1, markerGene2)))
+        assertThat(subject.getSortedMarkerGeneData(ImmutableSet.of(markerGene1, markerGene2)))
                 .extracting("y")
                 .containsOnly(0);
 
-        assertThat(subject.getMarkerGeneHeatmapDataSortedNaturally(ImmutableSet.of(markerGene1, markerGene2)))
+        assertThat(subject.getSortedMarkerGeneData(ImmutableSet.of(markerGene1, markerGene2)))
                 .extracting("cellGroupValueWhereMarker")
                 .containsOnly(cellGroupValueWhereMarker2);
     }
@@ -167,7 +167,7 @@ void whenMarkerGeneHasNumericalClusterNames_theySortedCorrectly() {
                 MarkerGene.create(randomGeneIds.get(2), "1", cellGroupValueWhereMarkers[2], 0.001,
                         "6", 1000, 10000, EXPRESSION_UNIT));
 
-        var result = subject.getMarkerGeneHeatmapDataSortedNaturally(markerGenes);
+        var result = subject.getSortedMarkerGeneData(markerGenes);
         assertThat(result).hasSize(3);
 
         assertThat(result).element(0).extracting("cellGroupValueWhereMarker")
@@ -204,7 +204,7 @@ void whenMarkerGeneHasNotOnlyNumericalClusterNames_theySortedCorrectly() {
                 MarkerGene.create(randomGeneIds.get(3), "1", cellGroupValueWhereMarkers[3], 0.001,
                         "6", 1000, 10000, EXPRESSION_UNIT));
 
-        var result = subject.getMarkerGeneHeatmapDataSortedNaturally(markerGenes);
+        var result = subject.getSortedMarkerGeneData(markerGenes);
         assertThat(result).hasSize(4);
 
         assertThat(result).element(0).extracting("cellGroupValueWhereMarker")
@@ -243,7 +243,7 @@ void whenMarkerGeneHasOnlyStringClusterNames_theySortedCorrectly() {
                 MarkerGene.create(randomGeneIds.get(3), "1", cellGroupValueWhereMarkers[3], 0.001,
                         "6", 1000, 10000, EXPRESSION_UNIT));
 
-        var result = subject.getMarkerGeneHeatmapDataSortedNaturally(markerGenes);
+        var result = subject.getSortedMarkerGeneData(markerGenes);
         assertThat(result).hasSize(4);
 
         assertThat(result).element(0).extracting("cellGroupValueWhereMarker")

diff --git a/.../uk/ac/ebi/atlas/search/metadata/JsonMultiexperimentCellTypeMarkerGenesControllerWIT.java b/.../uk/ac/ebi/atlas/search/metadata/JsonMultiexperimentCellTypeMarkerGenesControllerWIT.java
@@ -14,7 +14,6 @@
 import org.springframework.web.context.WebApplicationContext;
 import uk.ac.ebi.atlas.configuration.TestConfig;
 
-import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
 import java.util.Base64;