Encode hubs #1800
Encode hubs #1800
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for two new data hub types (4DN and ENCODE Contact Maps/HiC) to the IGV browser, along with important refactoring to improve track property handling and attribute normalization across different data sources.
- Added support for 4DN data hub with menu integration and file list URL configuration
- Added ENCODE Contact Maps (HiC) data support with whole-genome resolution handling
- Refactored TrackProperties to use boxed types (Integer, Float, Boolean) instead of primitives to properly handle unset values and avoid default value issues
- Implemented attribute name normalization to unify different naming conventions across UCSC, ENCODE, and 4DN data sources
- Created CaseInsensitiveMap utility class for case-insensitive chromosome lookups in HiC files
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/igv/util/collections/CaseInsensitiveMap.java | New utility class implementing case-insensitive Map for chromosome name lookups |
| src/main/java/org/igv/track/TrackProperties.java | Refactored fields from primitives to boxed types (Integer, Float, Boolean) to properly handle null/unset values |
| src/main/java/org/igv/track/AbstractTrack.java | Updated setProperties() to handle nullable TrackProperties fields with proper null checks |
| src/main/java/org/igv/ui/action/BrowseEncodeAction.java | Added new hub types (HIC, FOUR_DN), attribute normalization logic, and track line construction from attributes |
| src/main/java/org/igv/ui/IGVMenuBar.java | Added menu items for ENCODE Contact Maps and 4DN data hub |
| src/main/java/org/igv/encode/EncodeTrackChooserFactory.java | Added support for 4DN and HiC file lists, updated filtering and path column handling |
| src/main/java/org/igv/hic/HicFile.java | Implemented CaseInsensitiveMap for chromosome lookups and added getWGResolution() for whole-genome resolution |
| src/main/java/org/igv/bedpe/HicSource.java | Updated getBinSize() to handle whole-genome "all" chromosome with special resolution |
| src/main/java/org/igv/prefs/IGVPreferences.java | Simplified preference retrieval by removing redundant methods and improving error messages |
| src/main/resources/preferences.tab | Added FOUR_DN_FILELIST_URL configuration constant |
| src/main/java/org/igv/prefs/Constants.java | Added FOUR_DN_FILELIST_URL constant definition |
| src/main/java/org/igv/ui/color/ColorUtilities.java | Added trim() calls when parsing RGB color values to handle whitespace |
| src/test/java/org/igv/util/ParsingUtilsTest.java | Fixed test to use getAutoScale() instead of deprecated isAutoScale() |
| src/main/java/org/igv/track/DataTrack.java | Added clarifying comment about cache purging behavior |
| src/main/java/org/igv/track/DataSourceTrack.java | Removed commented-out dead code and extra whitespace |
| src/main/java/org/igv/encode/TrackChooserModel.java | Simplified selection logic and added fireTableDataChanged() call |
| src/main/java/org/igv/encode/TrackChooser.java | Added updateRecordSelectionState() method to sync loaded track states |
| src/main/java/org/igv/encode/FileRecord.java | Removed unused getAttributeNames() method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static Logger log = LogManager.getLogger(EncodeTrackChooserFactory.class); | ||
|
|
||
| private static Map<String, TrackChooser> instanceMap = Collections.synchronizedMap(new HashMap<>()); | ||
| private static NumberFormatter numberFormatter = new NumberFormatter(); |
There was a problem hiding this comment.
Unused field declaration. The numberFormatter field on line 35 is declared but never used in the file. Consider removing it to reduce unnecessary code.
| public Set<String> keySet() { | ||
| return map.keySet(); |
There was a problem hiding this comment.
The keySet() method returns keys in lowercase format, which may not be the expected behavior. When a user calls keySet(), they typically expect to get back the original keys as they were inserted, not the lowercased internal representation. Consider storing both the original key and the lowercase version internally, and returning the original keys in keySet(), or clearly document this behavior limitation in the class JavaDoc.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| private Boolean itemRGB = null; | ||
|
|
||
| private boolean useScore = false; | ||
| private Boolean useScore = null; |
There was a problem hiding this comment.
Inconsistent initialization of Boolean fields. Line 109 initializes itemRGB to null explicitly (private Boolean itemRGB = null;), while line 111 does the same for useScore (private Boolean useScore = null;). These explicit null initializations are redundant since object fields default to null. Additionally, line 89 has autoscale without explicit initialization. For consistency and clarity, either remove the explicit = null or apply it consistently to all nullable Boolean fields.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
No description provided.