diff --git a/scripts/patch_output_cif_files.py b/scripts/patch_output_cif_files.py index 0094df4f..ac9d4e80 100644 --- a/scripts/patch_output_cif_files.py +++ b/scripts/patch_output_cif_files.py @@ -164,9 +164,9 @@ def extract_rcsb_id(cif_path: Path, rcsb_regex: str) -> str | None: """Extract and validate the RCSB id from a cif path. ``rcsb_regex`` locates the candidate (it must contain exactly one capturing group around - the id; see ``--rcsb-pattern``). The id must be a **complete folder component**: a - capture that is only the prefix of a longer folder name is rejected, so a stray suffix - can't silently resolve to the wrong entry. The whole-component token is then checked + the id; see ``--rcsb-pattern``). The id must **start a folder component**, and must + either fill that component or be followed by a delimiter the pattern itself matches -- + so a stray prefix can't silently resolve to the wrong entry. The token is then checked against the PDB-id grammar and returned *verbatim* (legacy ``4hhb`` or extended ``pdb_00004hhb`` -- no normalization). @@ -180,11 +180,19 @@ def extract_rcsb_id(cif_path: Path, rcsb_regex: str) -> str | None: 1abc_pdb_1000abcd -> None (two id-like parts -- ambiguous) logs -> None (not a PDB id) - Possible problem this guards against: without the whole-component rule, ``4hhb_final`` - would yield ``4hhb`` and ``1abc_pdb_1000abcd`` would yield ``1abc`` -- both silently - patching the wrong entry. Such folders are skipped with a warning instead. If your - folders embed the id in a larger name, pass a custom ``--rcsb-pattern`` (beware names - with more than one id-like substring). + Possible problem this guards against: without the rule, ``4hhb_final`` would yield + ``4hhb`` and ``1abc_pdb_1000abcd`` would yield ``1abc`` -- both silently patching the + wrong entry. Such folders are skipped with a warning instead. + + When the id is genuinely embedded in a longer folder name, say so in the pattern by + matching the delimiter after the capturing group. Occupancy-sweep runs produce folders + like ``1VME_0.25occA_0.75occB`` whose reference entry really is ``1VME``:: + + --rcsb-pattern '/([0-9][A-Za-z0-9]{3}|pdb_[A-Za-z0-9]{8})_[0-9.]+occ' + + Matching past the group is what distinguishes "I know what follows the id" from the + default pattern's "the id is the whole folder", so relaxing it here does not weaken the + default. Beware names containing more than one id-like substring. Returns ``None`` when no complete PDB-id folder is found. Raises ``InvalidRcsbIdError`` when a whole component is captured but is not a valid PDB id (a likely sign the pattern @@ -202,12 +210,18 @@ def extract_rcsb_id(cif_path: Path, rcsb_regex: str) -> str | None: m = rcsb_re.search(path_str) if not m: return None - # The id must be a whole folder component: reject a capture that is only a prefix/suffix of - # a longer name (e.g. "4hhb" out of "4hhb_final" or "4hhb" out of "foo4hhb"), which would - # silently resolve to the wrong entry. A whole-component id is bounded by path separators (or - # the start/end of the path). + # The id must start a folder component: reject a capture that begins mid-name + # (e.g. "4hhb" out of "foo4hhb"), which would silently resolve to the wrong entry. start_ok = m.start(1) == 0 or path_str[m.start(1) - 1] == "/" - end_ok = m.end(1) == len(path_str) or path_str[m.end(1)] == "/" + # For the right-hand edge the id must either fill the component, or the pattern itself + # must say what follows it. A pattern that matches past its capturing group has stated + # the delimiter explicitly (e.g. `([0-9][A-Za-z0-9]{3})_[0-9.]+occ` for occupancy-sweep + # folders like `1VME_0.25occA_0.75occB`), which is the documented way to handle ids + # embedded in longer names. With the default pattern nothing follows the group, so the + # strict rule still applies and `4hhb_final` is still skipped rather than silently + # patched as `4hhb`. + pattern_states_delimiter = m.end(0) > m.end(1) + end_ok = pattern_states_delimiter or m.end(1) == len(path_str) or path_str[m.end(1)] == "/" if not (start_ok and end_ok): return None token = m.group(1) diff --git a/tests/eval/test_patch_output_cif_files.py b/tests/eval/test_patch_output_cif_files.py index 141eed8d..177ec693 100644 --- a/tests/eval/test_patch_output_cif_files.py +++ b/tests/eval/test_patch_output_cif_files.py @@ -57,7 +57,9 @@ def test_extract_rcsb_id_from_folder(script, folder: str, expected: str | None) ``1abc_pdb_1000abcd`` embeds an id-shaped substring, and without the whole-component rule the extractor would silently capture ``4hhb`` / ``1abc`` and patch the wrong entry. These are skipped instead. Extracting an id embedded inside a larger folder name is deliberately - NOT supported by the default pattern (it is ambiguous when several id-like parts appear). + NOT supported by the default pattern (it is ambiguous when several id-like parts appear); + a pattern that names the delimiter explicitly can opt in -- see + ``test_pattern_stating_delimiter_extracts_embedded_id``. """ path = Path(f"/data/results/grid_search_results/{folder}/trial_1/refined.cif") assert script.extract_rcsb_id(path, _DEFAULT_REGEX) == expected @@ -95,3 +97,38 @@ def test_extract_rcsb_id_requires_single_group(script, bad_regex: str) -> None: path = Path("/data/results/grid_search_results/pdb_00004hhb/refined.cif") with pytest.raises(ValueError, match="exactly one capturing group"): script.extract_rcsb_id(path, bad_regex) + + +def test_pattern_stating_delimiter_extracts_embedded_id(script) -> None: + """A pattern that matches past its capturing group opts into an embedded id. + + Occupancy-sweep grid searches emit one folder per (entry, occupancy) pair -- + ``1VME_0.25occA_0.75occB`` -- whose reference entry really is ``1VME``: the inputs tree + stores it at ``processed/1VME/1VME_single_001_density_input.cif``. The default pattern + rejects these because the id does not fill the folder component, and before this was + supported no ``--rcsb-pattern`` could rescue them: the whole-component rule was applied + after the match, so capturing the prefix was rejected and capturing the whole folder + failed id validation. Matching the delimiter after the group is how the caller states + that it knows what follows the id. + """ + occ = r"results/([0-9][A-Za-z0-9]{3}|pdb_[A-Za-z0-9]{8})_[0-9.]+occ" + for folder, expected in [ + ("1VME_0.25occA_0.75occB", "1VME"), + ("1VME_1.0occA", "1VME"), + ("9BN8_1.0occB", "9BN8"), + ("2A26_0.5occA_0.5occB", "2A26"), + ]: + path = Path(f"/data/results/{folder}/protpardelle_X-RAY_DIFFRACTION/ens8/refined.cif") + assert script.extract_rcsb_id(path, occ) == expected + + +def test_default_pattern_still_rejects_embedded_ids(script) -> None: + """Relaxing the rule for delimiter-stating patterns must not relax the default. + + Nothing follows the capturing group in the default pattern, so the strict + whole-component rule still applies and these stay skipped rather than silently + patching the wrong entry. + """ + for folder in ("4hhb_final", "1abc_pdb_1000abcd", "4hhb_0.5occA"): + path = Path(f"/data/results/grid_search_results/{folder}/trial_1/refined.cif") + assert script.extract_rcsb_id(path, _DEFAULT_REGEX) is None