Skip to content

Commit

Permalink
[ALS-5787] Open Access StatViz is not showing "Other" (#180)
Browse files Browse the repository at this point in the history
* Update key length handling in limitKeySize

The limitKeySize method in the VisualizationUtil class has been updated to ensure key uniqueness when keys exceed a certain length. A more robust method of shortening the keys has been implemented: if the key is longer than 45 characters, it is cut off and replaced with "..." and additional characters are appended to ensure uniqueness if needed. This helps limit key size while preserving uniqueness.

* Add VisualizationUtilTests and refine key shortening logic

Added a new test class, VisualizationUtilTests to validate the functionality of the VisualizationUtil class. Adjusted the 'limitKeySize' method to ensure key uniqueness when keys are shortened to a maximum length of 45 characters. If shortened keys are not unique, additional trailing characters are included until uniqueness is achieved.

* Add check on length before substring

* Update pic-sure-util/src/main/java/edu/harvard/dbmi/avillach/util/VisualizationUtil.java

* Update pic-sure-util/src/main/java/edu/harvard/dbmi/avillach/util/VisualizationUtil.java

* Update pic-sure-util/src/main/java/edu/harvard/dbmi/avillach/util/VisualizationUtil.java

* Update pic-sure-resources/pic-sure-visualization-resource/src/test/java/edu/harvard/hms/dbmi/avillach/resource/visualization/service/VisualizationUtilTests.java

Co-authored-by: Luke Sikina <lucas.sikina@gmail.com>

* Fix import for modified unit test

* Refactor and add test cases for key size limiting function

Refactors existing code by simplifying key size limiting logic in VisualizationUtil and added several new unit tests to ensure its correct behavior with different input scenarios including long keys, empty maps, null maps, and uniqueness near middle.

* Update null handling in VisualizationUtil's limitKeySize

Changed the handling of null input in VisualizationUtil's limitKeySize from returning a new HashMap to throwing an IllegalArgumentException. Also, updated the corresponding test to check for this exception instead of comparing with an empty map.

---------

Co-authored-by: Luke Sikina <lucas.sikina@gmail.com>
  • Loading branch information
Gcolon021 and Luke-Sikina committed Feb 21, 2024
1 parent 8ecd080 commit 6733173
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package edu.harvard.hms.dbmi.avillach.resource.visualization.service;

import edu.harvard.dbmi.avillach.util.VisualizationUtil;
import org.junit.Test;
import org.junit.jupiter.api.DisplayName;

import java.util.HashMap;
import java.util.Map;

import static org.junit.Assert.assertEquals;

public class VisualizationUtilTests {

@Test
@DisplayName("Test limitKeySize")
public void testLimitKeySizeUniqueness() {
Map<String, Integer> axisMap = new HashMap<>(Map.of(
"Disease-Specific (Asthma, Allergy and Inflammation, PUB)", 1,
"Disease-Specific (Asthma, Allergy and Inflammation, PUB, NPU)", 1,
"Disease-Specific (Asthma, Allergy and Inflammation, NPU)", 1,
"Disease-Specific (Asthma, Allergy and Inflammation)", 1
));

Map<String, Integer> actual = VisualizationUtil.limitKeySize(axisMap);

Map<String, Integer> expected = Map.of(
"Disease-Specific (Asthma, Allergy an..., PUB)", 1,
"Disease-Specific (Asthma, Allergy an...ation)", 1,
"Disease-Specific (Asthma, Allergy an..., NPU)", 1,
"Disease-Specific (Asthma, Allergy a...B, NPU)", 1
);
assertEquals(expected, actual);
}

@Test
@DisplayName("Test Empty Map limitKeySize")
public void testEmptyMapLimitKeySize() {
Map<String, Integer> axisMap = new HashMap<>();
Map<String, Integer> actual = VisualizationUtil.limitKeySize(axisMap);
Map<String, Integer> expected = new HashMap<>();
assertEquals(expected, actual);
}

@Test
@DisplayName("Test null Map limitKeySize")
public void testNullMapLimitKeySize() {
Map<String, Integer> axisMap = null;
// this should throw a NullPointerException
try {
VisualizationUtil.limitKeySize(axisMap);
} catch (IllegalArgumentException e) {
assertEquals("axisMap cannot be null", e.getMessage());
}
}

@Test
@DisplayName("Test with no long keys limitKeySize")
public void testNoLongKeysLimitKeySize() {
// Test with no long keys
Map<String, Integer> axisMap = new HashMap<>();
for (int i = 0; i < 10; i++) {
axisMap.put("key" + i, 1);
}
Map<String, Integer> actual = VisualizationUtil.limitKeySize(axisMap);
Map<String, Integer> expected = new HashMap<>(axisMap);
assertEquals(expected, actual);
}

@Test
@DisplayName("Test with keys of greater than 45 characters and uniqueness is near middle limitKeySize")
public void testKeysOfGreaterLengthAndUniquenessNearMiddleLimitKeySize() {
Map<String, Integer> axisMap = new HashMap<>();
axisMap.put("Hello, this is a long key that is STRING1 greater than 45 characters and is unique", 1);
axisMap.put("Hello, this is a long key that is STRING2 greater than 45 characters and is unique", 1);
axisMap.put("Hello, this is a long key that is STRING3 greater than 45 characters and is unique", 1);

Map<String, Integer> actual = VisualizationUtil.limitKeySize(axisMap);

// loop through the keys and check if they are less than 45 characters
for (String key : actual.keySet()) {
assertEquals(45, key.length());
}

Map<String, Integer> expected = Map.of(
"Hello, this is a long key that is ST...unique", 1,
"Hello, this is a long key that is S... unique", 1,
"Hello, this is a long key that is ...s unique", 1
);

assertEquals(expected, actual);
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -69,25 +69,50 @@ public static Map<String, Integer> doProcessResults(Map<String, Integer> axisMap
}

/**
* Replaces long column names with shorter version.
* This method is used to limit the size of the keys in the axisMap to a maximum of 45 characters. If the key is longer
* than 45 characters, it will be shortened to 45 characters and the last 3 characters will be replaced with "...".
* If the shortened key is not unique, we will create a unique one
* <p>
*
* @param axisMap
* @return
* @param axisMap - Map of the categories and their counts
* @return Map<String, Integer> - Map of the categories and their counts with the keys limited to 45 characters
*/
private static Map<String, Integer> limitKeySize(Map<String, Integer> axisMap) {
List<String> toRemove = new ArrayList<>();
Map<String, Integer> toAdd = new HashMap<>();
axisMap.keySet().forEach(key -> {
if (key.length() > MAX_X_LABEL_LINE_LENGTH) {
toRemove.add(key);
toAdd.put(
key.substring(0, MAX_X_LABEL_LINE_LENGTH - 3) + "...",
axisMap.get(key));
}
public static Map<String, Integer> limitKeySize(Map<String, Integer> axisMap) {
if (axisMap == null) {
throw new IllegalArgumentException("axisMap cannot be null");
}

Map<String, Integer> newAxisMap = new HashMap<>();
HashSet<String> keys = new HashSet<>();
axisMap.forEach((key, value) -> {
String adjustedKey = key.length() < MAX_X_LABEL_LINE_LENGTH ? key : createAdjustedKey(axisMap, keys, key);
newAxisMap.put(adjustedKey, value);
keys.add(adjustedKey);
});
toRemove.forEach(key -> axisMap.remove(key));
axisMap.putAll(toAdd);
return axisMap;
return newAxisMap;
}

private static String createAdjustedKey(Map<String, Integer> axisMap, HashSet<String> keys, String key) {
String keyPrefix = key.substring(0, MAX_X_LABEL_LINE_LENGTH);
return isKeyPrefixInAxisMap(axisMap, keyPrefix) ? generateUniqueKey(keys, key) : appendEllipsis(keyPrefix);
}

private static boolean isKeyPrefixInAxisMap(Map<String, Integer> axisMap, String keyPrefix) {
return axisMap.keySet().stream().anyMatch(k -> k.startsWith(keyPrefix));
}

private static String generateUniqueKey(HashSet<String> keys, String key) {
int countFromEnd = 6;
String proposedKey;
do {
proposedKey = String.format("%s...%s", key.substring(0, MAX_X_LABEL_LINE_LENGTH - 3 - countFromEnd), key.substring(key.length() - countFromEnd));
countFromEnd++;
} while (keys.contains(proposedKey));
return proposedKey;
}

private static String appendEllipsis(String keyPrefixAdjusted) {
return String.format("%s...", keyPrefixAdjusted);
}

}

0 comments on commit 6733173

Please sign in to comment.