From 6cb96336b496bb05089fe9cefd7adf8b0435ae8f Mon Sep 17 00:00:00 2001 From: Amr ALHOSSARY Date: Sun, 16 Aug 2026 21:42:21 -0400 Subject: [PATCH 1/2] Support the new ECOD distribution format ECOD redesigned its distribution at v294.1 and neither the version comment nor the column layout is recognised any more. The version comment changed from "#ECOD version develop291" to "# Version: v295", so parsedVersion stayed null and getVersion() fell back to the string that was requested. The columns changed from 15 (or 16 in develop291, which inserts unp_acc) to 23 and then 25, so every data line in the current release is rejected and getAllDomains() quietly returns an empty list. Files that declare a column header - every release since develop101 - are now read by column name rather than by position, which covers all four layouts at once and will not need revisiting the next time a column moves. Older files keep the positional path unchanged. Along with the columns: - manual_rep now holds True/False rather than MANUAL_REP/AUTO_NONREP - assembly_id is empty on every row, meaning what NOT_DOMAIN_ASSEMBLY meant - the ligand list moved to ligand_comp_ids - the last column is empty on four rows in five, so trailing empty fields are kept rather than discarded by split() - 53% of rows now describe domains found in AlphaFold models, which have no PDB entry and so cannot be an EcodDomain; they are counted and reported rather than logged as errors Parsing nothing at all is now reported as an error instead of returning an empty list silently, and the per-line NumberFormatException warning is capped like the others rather than printing a stack trace per row. getVersion() reads only the file header. Parsing the current 657 MB release in full to answer that question costs over a gigabyte of heap, which does not fit comfortably in the 1500 MB the test JVM is given. --- .../nbio/structure/ecod/EcodInstallation.java | 455 ++++++++++++++++-- 1 file changed, 404 insertions(+), 51 deletions(-) diff --git a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java index f4be5cd4f5..5bd59dd14b 100644 --- a/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java +++ b/biojava-structure/src/main/java/org/biojava/nbio/structure/ecod/EcodInstallation.java @@ -272,12 +272,41 @@ public void clear() { * * Note that this may differ from the version requested in the constructor * for the special case of "latest" + *

+ * Since 7.3.0 this reads only the file's header rather than parsing the whole + * file, so it no longer has the side effect of loading every domain. * @return the ECOD version * @throws IOException If an error occurs while downloading or parsing the file */ @Override public String getVersion() throws IOException { - ensureDomainsFileInstalled(); + domainsFileLock.readLock().lock(); + logger.trace("LOCK readlock"); + try { + if( parsedVersion != null ) { + return parsedVersion; + } + } finally { + logger.trace("UNLOCK readlock"); + domainsFileLock.readLock().unlock(); + } + + // The version is declared in the first few lines of the file, so read those rather + // than the millions of domain records behind them. The current release is 657 MB and + // holds nearly three million records; parsing it in full to answer this question + // costs over a gigabyte of heap and several seconds. + ensureDomainsFileDownloaded(); + + domainsFileLock.writeLock().lock(); + logger.trace("LOCK writelock"); + try { + if( parsedVersion == null ) { + parsedVersion = parseVersionOnly(); + } + } finally { + logger.trace("UNLOCK writelock"); + domainsFileLock.writeLock().unlock(); + } if( parsedVersion == null) { return requestedVersion; @@ -285,6 +314,30 @@ public String getVersion() throws IOException { return parsedVersion; } + /** + * Reads the version from the header of the local domains file without parsing the + * domains themselves. + * @return the version, or null if the header does not declare one + * @throws IOException if the file cannot be read + * @since 7.3.0 + */ + private String parseVersionOnly() throws IOException { + try( BufferedReader in = new BufferedReader(new FileReader(getDomainFile())) ) { + String line; + while( (line = in.readLine()) != null ) { + Matcher match = EcodParser.VERSION_RE.matcher(line); + if( match.matches() ) { + return match.group(1); + } + if( !line.startsWith("#") ) { + // past the header block; from v294.1 the column names are not commented + return null; + } + } + } + return null; + } + /** * Get the top-level ECOD server URL. Defaults to "http://prodata.swmed.edu" * @return the url to the ecod server @@ -325,6 +378,24 @@ public void setCacheLocation(String cacheLocation) { domainsFileLock.writeLock().unlock(); } + /** + * Ensures the domains file is present and current locally, without parsing it. + * @throws IOException in cases of file I/O, including failure to download a healthy file + * @since 7.3.0 + */ + private void ensureDomainsFileDownloaded() throws IOException { + domainsFileLock.writeLock().lock(); + logger.trace("LOCK writelock"); + try { + if( !domainsAvailable() ) { + downloadDomains(); + } + } finally { + logger.trace("UNLOCK writelock"); + domainsFileLock.writeLock().unlock(); + } + } + /** * Blocks until ECOD domains file has been downloaded and parsed. * @@ -549,6 +620,24 @@ Current version (1.4) contains the following columns: v1.2 - added f-group identifiers to fasta file, domain description file. ECODf identifiers now used when available for F-group name. Domain assemblies now represented by assembly uid in domain assembly status. v1.4 - added seqid_range and headers (develop101) +v1.6 - renamed column 4 from f_id to t_id and inserted unp_acc (UniProt accession) as + column 9, giving 16 columns (seen in develop291) + +From v294.1 the distribution was redesigned. The header comment changed from +"#ECOD version develop291" to "# Version: v294.1", the column header row is no longer +commented out, and the columns became: + + uid ecod_domain_id manual_rep f_id pdb chain pdb_range seqid_range architecture_name + x_name h_name t_name f_name assembly_id domain_id_short range_count arch_manual + x_manual h_manual t_manual f_manual valid_structure ligand_binding + +v295 appends ligand_comp_ids and ligand_pdbnum, for 25 columns. Also note that +manual_rep now holds True/False rather than MANUAL_REP/AUTO_NONREP, that assembly_id +and domain_id_short are empty on every row, that f_name is empty rather than +F_UNCLASSIFIED for unclassified domains, and that uid restarts from 0. + +Because the columns have been renamed, reordered and added to repeatedly, files that +declare a column header are read by column name rather than by position. */ /** String for unclassified F-groups */ @@ -561,10 +650,28 @@ Current version (1.4) contains the following columns: public static final String IS_REPRESENTATIVE = "MANUAL_REP"; /** Indicates not a manual representative */ public static final String NOT_REPRESENTATIVE = "AUTO_NONREP"; + /** + * Matches the comment declaring the version, which has taken two forms: + * {@code #ECOD version develop291} up to develop292, and {@code # Version: v295} + * from v294.1 onwards. + * @since 7.3.0 + */ + static final Pattern VERSION_RE = Pattern.compile( + "^\\s*#\\s*(?:ECOD\\s+)?version\\s*:?\\s*(\\S+).*", Pattern.CASE_INSENSITIVE); private List domains; private String version; + // prevent too many warnings; negative numbers print all warnings + private int warnIsDomainAssembly = 1; + private int warnHierarchicalFormat = 5; + private int warnNumberOfFields = 10; + private int warnNumberFormat = 10; + /** Data lines that could not be turned into a domain, for the summary at the end */ + private int skippedLines = 0; + /** Data lines describing a domain in a computed model rather than a PDB entry */ + private int modelLines = 0; + public EcodParser(String filename) throws IOException { this(new File(filename)); } @@ -584,30 +691,55 @@ private void parse(BufferedReader in) throws IOException { // Allocate plenty of space for ECOD as of 2015 ArrayList domainsList = new ArrayList<>(500000); - Pattern versionRE = Pattern.compile("^\\s*#.*ECOD\\s*version\\s+(\\S+).*"); Pattern commentRE = Pattern.compile("^\\s*#.*"); - // prevent too many warnings; negative numbers print all warnings - int warnIsDomainAssembly = 1; - int warnHierarchicalFormat = 5; - int warnNumberOfFields = 10; + ColumnLayout layout = null; String line = in.readLine(); int lineNum = 1; while( line != null ) { // Check for requestedVersion string - Matcher match = versionRE.matcher(line); + Matcher match = VERSION_RE.matcher(line); if(match.matches()) { // special requestedVersion comment this.version = match.group(1); + } else if( ColumnLayout.isColumnHeader(line) ) { + // The column names. Since the columns have been renamed, reordered and + // added to several times, later lines are read by name rather than by + // position wherever this header is present (develop101 onwards). + layout = ColumnLayout.fromHeader(line); + logger.debug("Read ECOD column header at line {}: {} columns",lineNum,layout.size()); } else { match = commentRE.matcher(line); if(match.matches()) { // ignore comments } else { - // data line - String[] fields = line.split("\t"); - if( fields.length == 13 || fields.length == 14 || fields.length == 15) { + // data line. The last column is frequently empty, so keep trailing + // empty fields rather than letting split() discard them. + String[] fields = line.split("\t", -1); + if( layout != null ) { + String pdb = layout.get(fields, "pdb"); + if( pdb != null && pdb.isEmpty() ) { + // From v294.1 the distribution also classifies domains + // found in computed (AlphaFold) models, which have no PDB + // entry and so cannot be represented by an EcodDomain. + modelLines++; + } else { + try { + EcodDomain domain = parseDomain(fields, layout, lineNum); + if(domain != null) { + domainsList.add(domain); + } else { + skippedLines++; + warnMissingColumns(lineNum); + } + } catch(IllegalArgumentException e) { + // includes NumberFormatException and an unusable PDB id + skippedLines++; + warnUnparseableLine(lineNum, e); + } + } + } else if( fields.length == 13 || fields.length == 14 || fields.length == 15) { try { int i = 0; // field number, to allow future insertion of fields @@ -620,32 +752,16 @@ private void parse(BufferedReader in) throws IOException { // Manual column may be missing in version 1.0 files Boolean manual = null; if( fields.length >= 14) { - String manualString = fields[i++]; - if(manualString.equalsIgnoreCase(IS_REPRESENTATIVE)) { - manual = true; - } else if(manualString.equalsIgnoreCase(NOT_REPRESENTATIVE)) { - manual = false; - } else { - logger.warn("Unexpected value for manual field: {} in line {}",manualString,lineNum); - } + manual = parseManualRep(fields[i++], lineNum); } //Column 4: ECOD hierachy identifier - [X-group].[H-group].[T-group].[F-group] // hierarchical field, e.g. "1.1.4.1" - String[] xhtGroup = fields[i++].split("\\."); - if(xhtGroup.length < 3 || 4 < xhtGroup.length) { - if(warnHierarchicalFormat > 1) { - logger.warn("Unexpected format for hierarchical field \"{}\" in line {}",fields[i-1],lineNum); - warnHierarchicalFormat--; - } else if(warnHierarchicalFormat != 0) { - logger.warn("Unexpected format for hierarchical field \"{}\" in line {}. Not printing future similar warnings.",fields[i-1],lineNum); - warnHierarchicalFormat--; - } - } - Integer xGroup = xhtGroup.length>0 ? Integer.parseInt(xhtGroup[0]) : null; - Integer hGroup = xhtGroup.length>1 ? Integer.parseInt(xhtGroup[1]) : null; - Integer tGroup = xhtGroup.length>2 ? Integer.parseInt(xhtGroup[2]) : null; - Integer fGroup = xhtGroup.length>3 ? Integer.parseInt(xhtGroup[3]) : null; + Integer[] xhtfGroup = parseHierarchy(fields[i++], lineNum); + Integer xGroup = xhtfGroup[0]; + Integer hGroup = xhtfGroup[1]; + Integer tGroup = xhtfGroup[2]; + Integer fGroup = xhtfGroup[3]; //Column 5: PDB identifier String pdbId = fields[i++]; @@ -699,32 +815,18 @@ private void parse(BufferedReader in) throws IOException { assemblyId = Long.parseLong(assemblyStr); } - String ligandStr = fields[i++]; - Set ligands = null; - if( "NO_LIGANDS_4A".equals(ligandStr) || ligandStr.isEmpty() ) { - ligands = Collections.emptySet(); - } else { - String[] ligSplit = ligandStr.split(","); - ligands = new LinkedHashSet<>(ligSplit.length); - for(String s : ligSplit) { - ligands.add(s.intern()); - } - } + Set ligands = parseLigands(fields[i++]); EcodDomain domain = new EcodDomain(uid, domainId, manual, xGroup, hGroup, tGroup, fGroup,pdbId, chainId, range, seqId, architectureName, xGroupName, hGroupName, tGroupName, fGroupName, assemblyId, ligands); domainsList.add(domain); } catch(NumberFormatException e) { - logger.warn("Error in ECOD parsing at line "+lineNum,e); + skippedLines++; + warnUnparseableLine(lineNum, e); } } else { - if(warnNumberOfFields > 1) { - logger.warn("Unexpected number of fields in line {}.",lineNum); - warnNumberOfFields--; - } else if(warnNumberOfFields == 0) { - logger.warn("Unexpected number of fields in line {}. Not printing future similar warnings",lineNum); - warnIsDomainAssembly--; - } + skippedLines++; + warnMissingColumns(lineNum); } } } @@ -737,6 +839,23 @@ private void parse(BufferedReader in) throws IOException { else logger.info("Parsed {} ECOD domains from version {}",domainsList.size(),this.version); + if(modelLines > 0) { + logger.info("Ignored {} ECOD domains classified from computed models, " + + "which have no PDB entry", modelLines); + } + + if(domainsList.isEmpty() && skippedLines > 0) { + // Returning an empty list quietly is how an upstream format change went + // unnoticed for eight months. Say so instead. + logger.error("Parsed no ECOD domains from {} data lines of version {}. " + + "The file format has probably changed; please report this at " + + "https://github.com/biojava/biojava/issues", skippedLines, + this.version == null ? "unknown" : this.version); + } else if(skippedLines > 0) { + logger.warn("Skipped {} of {} ECOD data lines that could not be parsed", + skippedLines, skippedLines + domainsList.size()); + } + this.domains = Collections.unmodifiableList( domainsList ); @@ -747,6 +866,169 @@ private void parse(BufferedReader in) throws IOException { } } + /** + * Builds a domain from a data line using the column names the file declares in its + * header, rather than fixed offsets. This is what allows one parser to read the + * 15-column develop101 layout, the 16-column develop291 layout (which inserts + * {@code unp_acc}) and the 23- and 25-column v294.1 and v295 layouts. + * @param fields the tab-separated values of one data line + * @param layout the column names read from the file's header + * @param lineNum the line number, for warnings + * @return the domain, or null if the line does not carry every required column + * @throws NumberFormatException if a numeric column does not hold a number + * @since 7.3.0 + */ + private EcodDomain parseDomain(String[] fields, ColumnLayout layout, int lineNum) { + String uidStr = layout.get(fields, "uid"); + String domainId = layout.get(fields, "ecod_domain_id"); + // renamed from t_id to f_id when the hierarchy gained a fourth level + String hierarchy = layout.get(fields, "f_id", "t_id"); + String pdbId = layout.get(fields, "pdb"); + String chainId = layout.get(fields, "chain"); + String range = layout.get(fields, "pdb_range"); + if( uidStr == null || domainId == null || hierarchy == null + || pdbId == null || chainId == null || range == null ) { + return null; + } + + Long uid = Long.parseLong(uidStr); + Boolean manual = parseManualRep(layout.get(fields, "manual_rep"), lineNum); + Integer[] xhtfGroup = parseHierarchy(hierarchy, lineNum); + // absent before version 1.4 + String seqId = layout.get(fields, "seqid_range"); + + String architectureName = internName(layout.get(fields, "architecture_name", "arch_name")); + String xGroupName = internName(layout.get(fields, "x_name")); + String hGroupName = internName(layout.get(fields, "h_name")); + String tGroupName = internName(layout.get(fields, "t_name")); + // Up to develop292 an unclassified domain carried F_UNCLASSIFIED here. From + // v294.1 the name is simply empty, while f_id still classifies the domain to + // four levels, so the two are no longer equivalent and the empty value is + // deliberately left as it is rather than translated. + String fGroupName = internName(layout.get(fields, "f_name")); + + // v294.1 and later declare assembly_id but leave it empty on every row, which + // means the same as the NOT_DOMAIN_ASSEMBLY of earlier versions. + Long assemblyId = null; + String assemblyStr = layout.get(fields, "assembly_id", "asm_status"); + if( assemblyStr == null || assemblyStr.isEmpty() || NOT_DOMAIN_ASSEMBLY.equals(assemblyStr) ) { + assemblyId = uid; + } else if( IS_DOMAIN_ASSEMBLY.equals(assemblyStr) ) { + warnDomainAssembly(lineNum); + } else { + assemblyId = Long.parseLong(assemblyStr); + } + + // the ligand list moved from the last column to ligand_comp_ids in v295 + Set ligands = parseLigands(layout.get(fields, "ligand_comp_ids", "ligand")); + + return new EcodDomain(uid, domainId, manual, xhtfGroup[0], xhtfGroup[1], xhtfGroup[2], + xhtfGroup[3], pdbId, chainId, range, seqId, architectureName, xGroupName, + hGroupName, tGroupName, fGroupName, assemblyId, ligands); + } + + /** + * Reads the representative-status column, which held MANUAL_REP or AUTO_NONREP up to + * develop292 and True or False from v294.1 onwards. + * @return true, false, or null if the column is absent or unrecognised + * @since 7.3.0 + */ + private Boolean parseManualRep(String manualString, int lineNum) { + if(manualString == null) { + return null; + } + if(manualString.equalsIgnoreCase(IS_REPRESENTATIVE) || manualString.equalsIgnoreCase("true")) { + return true; + } + if(manualString.equalsIgnoreCase(NOT_REPRESENTATIVE) || manualString.equalsIgnoreCase("false")) { + return false; + } + logger.warn("Unexpected value for manual field: {} in line {}",manualString,lineNum); + return null; + } + + /** + * Splits the hierarchical identifier, e.g. "1.1.4.1". + * @return the X, H, T and F group numbers, any of which may be null if absent + * @since 7.3.0 + */ + private Integer[] parseHierarchy(String hierarchy, int lineNum) { + String[] xhtGroup = hierarchy.split("\\."); + if(xhtGroup.length < 3 || 4 < xhtGroup.length) { + if(warnHierarchicalFormat > 1) { + logger.warn("Unexpected format for hierarchical field \"{}\" in line {}",hierarchy,lineNum); + warnHierarchicalFormat--; + } else if(warnHierarchicalFormat != 0) { + logger.warn("Unexpected format for hierarchical field \"{}\" in line {}. Not printing future similar warnings.",hierarchy,lineNum); + warnHierarchicalFormat--; + } + } + Integer[] groups = new Integer[4]; + for(int j = 0; j < groups.length && j < xhtGroup.length; j++) { + groups[j] = Integer.parseInt(xhtGroup[j]); + } + return groups; + } + + /** + * Reads a comma-separated list of non-polymer entities close to the domain. + * @return the ligands, or an empty set for NO_LIGANDS_4A, an empty value or no column + * @since 7.3.0 + */ + private Set parseLigands(String ligandStr) { + if( ligandStr == null || ligandStr.isEmpty() || "NO_LIGANDS_4A".equals(ligandStr) ) { + return Collections.emptySet(); + } + String[] ligSplit = ligandStr.split(","); + Set ligands = new LinkedHashSet<>(ligSplit.length); + for(String s : ligSplit) { + ligands.add(s.intern()); + } + return ligands; + } + + /** + * Interns a name likely to be shared by many domains, stripping the quotes that + * versions up to develop292 wrapped some of them in. + * @since 7.3.0 + */ + private String internName(String name) { + if(name == null) { + return null; + } + return clearStringQuotes(name).intern(); + } + + private void warnDomainAssembly(int lineNum) { + if(warnIsDomainAssembly > 1) { + logger.info("Deprecated 'IS_DOMAIN_ASSEMBLY' value ignored in line {}.",lineNum); + warnIsDomainAssembly--; + } else if(warnIsDomainAssembly == 0) { + logger.info("Deprecated 'IS_DOMAIN_ASSEMBLY' value ignored in line {}. Not printing future similar warnings.",lineNum); + warnIsDomainAssembly--; + } + } + + private void warnMissingColumns(int lineNum) { + if(warnNumberOfFields > 1) { + logger.warn("Unexpected number of fields in line {}.",lineNum); + warnNumberOfFields--; + } else if(warnNumberOfFields == 1) { + logger.warn("Unexpected number of fields in line {}. Not printing future similar warnings",lineNum); + warnNumberOfFields--; + } + } + + private void warnUnparseableLine(int lineNum, IllegalArgumentException e) { + if(warnNumberFormat > 1) { + logger.warn("Error in ECOD parsing at line "+lineNum,e); + warnNumberFormat--; + } else if(warnNumberFormat == 1) { + logger.warn("Error in ECOD parsing at line "+lineNum+". Not printing future similar warnings",e); + warnNumberFormat--; + } + } + private String clearStringQuotes(String name) { if ( name.startsWith("\"")) name = name.substring(1); @@ -770,6 +1052,77 @@ public List getDomains() { public String getVersion() { return version; } + + /** + * Maps the column names an ECOD domain file declares in its header onto their + * positions, so a data line can be read by name rather than by offset. + *

+ * Every distribution since develop101 carries such a header. It is commented + * (#uid<tab>ecod_domain_id<tab>...) up to develop292 and + * uncommented (uid<tab>ecod_domain_id<tab>...) from v294.1 + * onwards. Because names have also been changed between versions, lookups accept + * aliases and any name the file does not declare simply reads as absent. + * + * @author Amr ALHOSSARY + * @since 7.3.0 + */ + private static class ColumnLayout { + private final Map columns; + + private ColumnLayout(Map columns) { + this.columns = columns; + } + + /** + * @return true if this line names the columns rather than holding domain data + */ + public static boolean isColumnHeader(String line) { + int tab = line.indexOf('\t'); + if(tab < 0) { + return false; + } + String first = line.substring(0, tab).trim(); + if(first.startsWith("#")) { + first = first.substring(1).trim(); + } + return first.equalsIgnoreCase("uid"); + } + + public static ColumnLayout fromHeader(String line) { + String[] names = line.split("\t", -1); + Map columns = new HashMap<>(names.length * 2); + for(int i = 0; i < names.length; i++) { + String name = names[i].trim(); + if(i == 0 && name.startsWith("#")) { + name = name.substring(1).trim(); + } + if(!name.isEmpty()) { + columns.put(name.toLowerCase(), i); + } + } + return new ColumnLayout(columns); + } + + /** + * @param fields the values of one data line + * @param aliases the names this column has gone by, most recent first + * @return the value of the first alias the file declares, or null if it declares + * none of them or this line is too short to reach it + */ + public String get(String[] fields, String... aliases) { + for(String alias : aliases) { + Integer i = columns.get(alias); + if(i != null) { + return i < fields.length ? fields[i] : null; + } + } + return null; + } + + public int size() { + return columns.size(); + } + } } From 2ad8e09e8140132196b3999c57f38cb1faab66df Mon Sep 17 00:00:00 2001 From: Amr ALHOSSARY Date: Sun, 16 Aug 2026 21:42:31 -0400 Subject: [PATCH 2/2] Test every ECOD layout offline The distribution is 657 MB, so the only test that touched the current release was testVersion, which asserts on one string. Every other test pins develop204. That is why a wholesale format change surfaced as a single one-line assertion failure, eight months after it happened. EcodParserTest reads fixtures taken verbatim from the real files - 13, 15, 16, 23 and 25 columns, both version comment forms, a row ending in an empty column, an AlphaFold-derived row, and a malformed row - through the Reader constructor. Fifteen tests, no network, well under a second. testVersion additionally parses the first few thousand lines of the local file. That is enough to notice a column change without building the three million domains the whole file now holds. --- .../test/ecod/EcodInstallationTest.java | 40 +++ .../nbio/structure/ecod/EcodParserTest.java | 323 ++++++++++++++++++ 2 files changed, 363 insertions(+) create mode 100644 biojava-structure/src/test/java/org/biojava/nbio/structure/ecod/EcodParserTest.java diff --git a/biojava-integrationtest/src/test/java/org/biojava/nbio/structure/test/ecod/EcodInstallationTest.java b/biojava-integrationtest/src/test/java/org/biojava/nbio/structure/test/ecod/EcodInstallationTest.java index 8ab3f29fb3..f384f2ba65 100644 --- a/biojava-integrationtest/src/test/java/org/biojava/nbio/structure/test/ecod/EcodInstallationTest.java +++ b/biojava-integrationtest/src/test/java/org/biojava/nbio/structure/test/ecod/EcodInstallationTest.java @@ -22,8 +22,12 @@ import static org.junit.Assert.*; +import java.io.BufferedReader; import java.io.File; +import java.io.FileReader; import java.io.IOException; +import java.io.Reader; +import java.io.StringReader; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -47,6 +51,7 @@ import org.biojava.nbio.structure.ecod.EcodDomain; import org.biojava.nbio.structure.ecod.EcodFactory; import org.biojava.nbio.structure.ecod.EcodInstallation; +import org.biojava.nbio.structure.ecod.EcodInstallation.EcodParser; import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; @@ -277,12 +282,47 @@ public void testFilterByHierarchy() throws IOException { assertEquals(expected,actual); } + /** + * Checks that the current release can still be read. + *

+ * The version is read from the file's header without parsing the domains, so this + * additionally parses the first few thousand lines of the same file. That is enough to + * notice a column change — which is what ECOD did at v294.1, unnoticed for months — + * without building the three million domains the whole file now holds. + */ @Test public void testVersion() throws IOException { EcodDatabase ecod3 = EcodFactory.getEcodDatabase("latest"); String version = ecod3.getVersion(); assertNotNull(version); assertNotEquals("latest", version); + System.out.println("latest version of ECOD is "+version); + + File domainsFile = new File(((EcodInstallation) ecod3).getCacheLocation(), + "ecod.latest.domains.txt"); + assertTrue("No local copy of the domains file at "+domainsFile, domainsFile.exists()); + + EcodParser parser = new EcodParser(firstLines(domainsFile, 5000)); + assertEquals(version, parser.getVersion()); + assertFalse("No domains parsed from ECOD "+version + + "; the distribution format has probably changed", + parser.getDomains().isEmpty()); + } + + /** + * @return a reader over the first {@code maxLines} lines of the file + */ + private static Reader firstLines(File f, int maxLines) throws IOException { + StringBuilder head = new StringBuilder(); + try (BufferedReader in = new BufferedReader(new FileReader(f))) { + String line; + int n = 0; + while (n < maxLines && (line = in.readLine()) != null) { + head.append(line).append('\n'); + n++; + } + } + return new StringReader(head.toString()); } /** diff --git a/biojava-structure/src/test/java/org/biojava/nbio/structure/ecod/EcodParserTest.java b/biojava-structure/src/test/java/org/biojava/nbio/structure/ecod/EcodParserTest.java new file mode 100644 index 0000000000..295806e915 --- /dev/null +++ b/biojava-structure/src/test/java/org/biojava/nbio/structure/ecod/EcodParserTest.java @@ -0,0 +1,323 @@ +/* + * BioJava development code + * + * This code may be freely distributed and modified under the + * terms of the GNU Lesser General Public Licence. This should + * be distributed with the code. If you do not have a copy, + * see: + * + * http://www.gnu.org/copyleft/lesser.html + * + * Copyright for this code is held jointly by the individual + * authors. These should be listed in @author doc comments. + * + * For more information on the BioJava project and its aims, + * or to join the biojava-l mailing list, visit the home page + * at: + * + * http://www.biojava.org/ + */ +package org.biojava.nbio.structure.ecod; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.io.StringReader; +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; + +import org.biojava.nbio.structure.ecod.EcodInstallation.EcodParser; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +/** + * Checks that {@link EcodParser} reads every layout ECOD has distributed. + *

+ * The columns have been renamed, reordered and added to several times, and the version + * comment itself changed form at v294.1. Because the full distribution is 657 MB, none of + * that was covered by a test that could run in reasonable time, and a format change went + * unnoticed for months. These fixtures are taken verbatim from the real files, so the + * contract is pinned in milliseconds rather than by a download. + * + * @author Amr ALHOSSARY + * @since 7.3.0 + */ +class EcodParserTest { + + /** develop204, list format 1.5: 15 columns, commented header, quoted names. */ + private static final String DEVELOP204 = String.join("\n", + "#/data/ecod/database_versions/v204/ecod.develop204.domains.txt", + "#ECOD version develop204", + "#Domain list version 1.5", + "#Grishin lab (http://prodata.swmed.edu/ecod)", + "#uid\tecod_domain_id\tmanual_rep\tf_id\tpdb\tchain\tpdb_range\tseqid_range" + + "\tarch_name\tx_name\th_name\tt_name\tf_name\tasm_status\tligand", + "002137905\te6b4nA1\tAUTO_NONREP\t1.1.1\t6b4n\tA\tA:1-99\tA:1-99\tbeta barrels" + + "\t\"cradle loop barrel\"\t\"RIFT-related\"\t\"acid protease\"" + + "\tF_UNCLASSIFIED\tNOT_DOMAIN_ASSEMBLY\tCL,G53,NA"); + + /** develop291, list format 1.6: 16 columns, f_id renamed t_id, unp_acc inserted at 9. */ + private static final String DEVELOP291 = String.join("\n", + "#/data/ecod/database_versions/v291/ecod.develop291.domains.txt", + "#ECOD version develop291", + "#Domain list version 1.6", + "#Grishin lab (http://prodata.swmed.edu/ecod)", + "#uid\tecod_domain_id\tmanual_rep\tt_id\tpdb\tchain\tpdb_range\tseqid_range\tunp_acc" + + "\tarch_name\tx_name\th_name\tt_name\tf_name\tasm_status\tligand", + "000000267\te1udzA1\tMANUAL_REP\t1.1.1\t1udz\tA\tA:203-381\tA:4-182\tP12345" + + "\tbeta barrels\t\"cradle loop barrel\"\t\"RIFT-related\"\t\"acid protease\"" + + "\tF_UNCLASSIFIED\tNOT_DOMAIN_ASSEMBLY\tNO_LIGANDS_4A"); + + private static final String V295_COLUMNS = + "uid\tecod_domain_id\tmanual_rep\tf_id\tpdb\tchain\tpdb_range\tseqid_range" + + "\tarchitecture_name\tx_name\th_name\tt_name\tf_name\tassembly_id\tdomain_id_short" + + "\trange_count\tarch_manual\tx_manual\th_manual\tt_manual\tf_manual" + + "\tvalid_structure\tligand_binding\tligand_comp_ids\tligand_pdbnum"; + + /** v295: 25 columns, uncommented header, True/False, empty assembly_id, moved ligands. */ + private static final String V295 = String.join("\n", + "# ECOD Domain List", + "# Version: v295", + "# Generated: 2026-06-24 22:47:42", + "# Ligand cutoff: 4.0 A (NO_LIGANDS_4A = no contact within cutoff)", + "#", + V295_COLUMNS, + "0\te2nmzA1\tTrue\t1.1.1.3\t2nmz\tA\tA:1-99\tA:1-99\tbeta barrels\tcradle loop barrel" + + "\tRIFT-related\tacid protease\tRVP\t\t\t1\tFalse\tFalse\tFalse\tFalse\tTrue" + + "\tTrue\tTrue\tROC,SO4\tA:601,A:602,B:401", + // the last column is empty on four rows in five, so split() must keep it + "3\te2rspA1\tTrue\t1.1.1.3\t2rsp\tA\tA:1-124\tA:1-124\tbeta barrels\tcradle loop barrel" + + "\tRIFT-related\tacid protease\tRVP\t\t\t1\tFalse\tFalse\tFalse\tFalse\tTrue" + + "\tTrue\tFalse\tNO_LIGANDS_4A\t", + // a domain classified from an AlphaFold model: no PDB entry, so no EcodDomain + "3163557\tP44140_F1_nD2\tFalse\t2004.1.1.123\t\t\t131-315\t131-315\talpha bundles" + + "\tsomething\tsomething else\ta third thing\t\t\t\t1\tFalse\tFalse\tFalse" + + "\tFalse\tTrue\tTrue\tFalse\tNO_LIGANDS_4A\t"); + + private static List parse(String contents) throws IOException { + return new EcodParser(new StringReader(contents)).getDomains(); + } + + private static String version(String contents) throws IOException { + return new EcodParser(new StringReader(contents)).getVersion(); + } + + @Nested + class Version { + @Test + void oldHeaderForm() throws IOException { + assertEquals("develop204", version(DEVELOP204)); + assertEquals("develop291", version(DEVELOP291)); + } + + @Test + void newHeaderForm() throws IOException { + assertEquals("v295", version(V295)); + assertEquals("v294.1", version("# ECOD Domain List\n# Version: v294.1\n")); + } + + @Test + void listFormatVersionIsNotTheEcodVersion() throws IOException { + // "#Domain list version 1.5" describes the columns, not the release + assertNull(version("#Grishin lab\n#Domain list version 1.5\n")); + } + + @Test + void absentVersionIsNull() throws IOException { + assertNull(version("#Grishin lab (http://prodata.swmed.edu/ecod)\n")); + } + } + + @Nested + class OldFormats { + @Test + void listFormat15() throws IOException { + List domains = parse(DEVELOP204); + assertEquals(1, domains.size()); + EcodDomain d = domains.get(0); + assertEquals(Long.valueOf(2137905), d.getUid()); + assertEquals("e6b4nA1", d.getDomainId()); + assertEquals(Boolean.FALSE, d.getManual()); + assertEquals(Integer.valueOf(1), d.getXGroup()); + assertEquals(Integer.valueOf(1), d.getHGroup()); + assertEquals(Integer.valueOf(1), d.getTGroup()); + assertNull(d.getFGroup()); + assertEquals("6B4N", d.getPdbId().getId()); + assertEquals("A", d.getChainId()); + assertEquals("A:1-99", d.getRange()); + assertEquals("A:1-99", d.getSeqIdRange()); + assertEquals("beta barrels", d.getArchitectureName()); + // quotes were stripped up to develop292 + assertEquals("cradle loop barrel", d.getXGroupName()); + assertEquals("RIFT-related", d.getHGroupName()); + assertEquals("acid protease", d.getTGroupName()); + assertEquals("F_UNCLASSIFIED", d.getFGroupName()); + assertEquals(Long.valueOf(2137905), d.getAssemblyId()); + assertEquals(new LinkedHashSet<>(Arrays.asList("CL", "G53", "NA")), d.getLigands()); + } + + /** + * develop291 inserted unp_acc before arch_name. Read positionally, every field from + * there on shifts by one and the domain is silently mangled or dropped. + */ + @Test + void listFormat16InsertsUnpAcc() throws IOException { + List domains = parse(DEVELOP291); + assertEquals(1, domains.size()); + EcodDomain d = domains.get(0); + assertEquals(Boolean.TRUE, d.getManual()); + assertEquals("1UDZ", d.getPdbId().getId()); + assertEquals("A:4-182", d.getSeqIdRange()); + assertEquals("beta barrels", d.getArchitectureName()); + assertEquals("acid protease", d.getTGroupName()); + assertEquals("F_UNCLASSIFIED", d.getFGroupName()); + assertEquals(Long.valueOf(267), d.getAssemblyId()); + assertEquals(Collections.emptySet(), d.getLigands()); + } + + /** + * Headers were only added in develop101. Older files are still read by position. + */ + @Test + void thirteenColumnsWithoutAHeader() throws IOException { + List domains = parse(String.join("\n", + "#ECOD version develop45", + "000000001\te1udzA1\t1.1.1\t1udz\tA\tA:203-381\tbeta barrels" + + "\t\"cradle loop barrel\"\t\"RIFT-related\"\t\"acid protease\"" + + "\tF_UNCLASSIFIED\tNOT_DOMAIN_ASSEMBLY\tNO_LIGANDS_4A")); + assertEquals(1, domains.size()); + EcodDomain d = domains.get(0); + assertNull(d.getManual(), "no manual_rep column before list format 1.1"); + assertNull(d.getSeqIdRange(), "no seqid_range column before list format 1.4"); + assertEquals("1UDZ", d.getPdbId().getId()); + assertEquals("acid protease", d.getTGroupName()); + } + } + + @Nested + class NewFormat { + @Test + void twentyFiveColumns() throws IOException { + List domains = parse(V295); + // two PDB domains; the AlphaFold-derived row cannot be an EcodDomain + assertEquals(2, domains.size()); + + EcodDomain d = domains.get(0); + assertEquals(Long.valueOf(0), d.getUid()); + assertEquals("e2nmzA1", d.getDomainId()); + assertEquals(Boolean.TRUE, d.getManual(), "manual_rep is now True/False"); + assertEquals(Integer.valueOf(1), d.getXGroup()); + assertEquals(Integer.valueOf(1), d.getHGroup()); + assertEquals(Integer.valueOf(1), d.getTGroup()); + assertEquals(Integer.valueOf(3), d.getFGroup(), "f_id now carries a fourth level"); + assertEquals("2NMZ", d.getPdbId().getId()); + assertEquals("A", d.getChainId()); + assertEquals("A:1-99", d.getRange()); + assertEquals("A:1-99", d.getSeqIdRange()); + assertEquals("beta barrels", d.getArchitectureName()); + assertEquals("cradle loop barrel", d.getXGroupName()); + assertEquals("RIFT-related", d.getHGroupName()); + assertEquals("acid protease", d.getTGroupName()); + assertEquals("RVP", d.getFGroupName()); + // assembly_id is empty on every row of v294.1 and later, which means the same + // as the NOT_DOMAIN_ASSEMBLY of earlier versions + assertEquals(Long.valueOf(0), d.getAssemblyId()); + assertEquals(new LinkedHashSet<>(Arrays.asList("ROC", "SO4")), d.getLigands(), + "the ligand list moved to ligand_comp_ids"); + } + + /** + * ligand_pdbnum, the last column, is empty on four rows in five. String.split + * discards trailing empty fields unless asked not to, which would make those rows + * look one column short. + */ + @Test + void rowEndingInAnEmptyColumn() throws IOException { + EcodDomain d = parse(V295).get(1); + assertEquals("e2rspA1", d.getDomainId()); + assertEquals("2RSP", d.getPdbId().getId()); + assertEquals(Collections.emptySet(), d.getLigands()); + } + + @Test + void twentyThreeColumnsOfV2941() throws IOException { + List domains = parse(String.join("\n", + "# ECOD Domain List", + "# Version: v294.1", + "#", + "uid\tecod_domain_id\tmanual_rep\tf_id\tpdb\tchain\tpdb_range\tseqid_range" + + "\tarchitecture_name\tx_name\th_name\tt_name\tf_name\tassembly_id" + + "\tdomain_id_short\trange_count\tarch_manual\tx_manual\th_manual" + + "\tt_manual\tf_manual\tvalid_structure\tligand_binding", + "1\te1hvcA1\tFalse\t1.1.1.3\t1hvc\tA\tA:1B-99A\tA:1-203\tbeta barrels" + + "\tcradle loop barrel\tRIFT-related\tacid protease\tRVP\t\t\t\tFalse" + + "\tFalse\tFalse\tFalse\tFalse\tTrue\tFalse")); + assertEquals(1, domains.size()); + EcodDomain d = domains.get(0); + assertEquals("1HVC", d.getPdbId().getId()); + assertEquals("A:1B-99A", d.getRange()); + assertEquals("RVP", d.getFGroupName()); + // there is no ligand column at all in v294.1 + assertEquals(Collections.emptySet(), d.getLigands()); + } + + @Test + void columnHeaderIsNotADomain() throws IOException { + // v294.1 stopped commenting the column names out, so they arrive looking like data + for (EcodDomain d : parse(V295)) { + assertFalse("uid".equals(d.getDomainId())); + } + } + + /** + * An empty f_name is not the same as F_UNCLASSIFIED: f_id still classifies the + * domain to four levels, so the empty value is left as it is rather than translated. + */ + @Test + void emptyFGroupNameIsLeftAlone() throws IOException { + List domains = parse(String.join("\n", + "# Version: v295", + V295_COLUMNS, + "7\te4fivA1\tTrue\t1.1.1.3\t4fiv\tA\tA:4-116\tA:1-113\tbeta barrels" + + "\tcradle loop barrel\tRIFT-related\tacid protease\t\t\t\t1\tFalse" + + "\tFalse\tFalse\tFalse\tTrue\tTrue\tTrue\tLP1\tA:201")); + assertEquals(1, domains.size()); + assertEquals("", domains.get(0).getFGroupName()); + } + } + + @Nested + class Robustness { + @Test + void unparseableLinesAreSkippedNotFatal() throws IOException { + List domains = parse(String.join("\n", + "# Version: v295", + V295_COLUMNS, + "not-a-number\tefoo\tTrue\t1.1.1.3\tfoo1\tA\tA:1-9\tA:1-9\ta\tb\tc\td\te" + + "\t\t\t1\tFalse\tFalse\tFalse\tFalse\tTrue\tTrue\tFalse\t\t", + "0\te2nmzA1\tTrue\t1.1.1.3\t2nmz\tA\tA:1-99\tA:1-99\tbeta barrels" + + "\tcradle loop barrel\tRIFT-related\tacid protease\tRVP\t\t\t1" + + "\tFalse\tFalse\tFalse\tFalse\tTrue\tTrue\tTrue\tROC,SO4\tA:601")); + assertEquals(1, domains.size(), "the good line is still read"); + assertEquals("e2nmzA1", domains.get(0).getDomainId()); + } + + @Test + void shortLineIsSkipped() throws IOException { + assertTrue(parse(String.join("\n", + "# Version: v295", + V295_COLUMNS, + "0\te2nmzA1\tTrue")).isEmpty()); + } + + @Test + void emptyFileYieldsNoDomains() throws IOException { + assertTrue(parse("").isEmpty()); + } + } +}