Merge pull request #38 from lamalab-org/unifying-branches

Unifying branches

Author: r-fedorov

src/openclatura/assembly_parts.py

@@ -27,6 +27,17 @@ class NameTokenBinding:
     right_context: str = ""
 
 
+class RenderedSubstituentName(str):
+    """A rendered substituent carrying construction-time boundary metadata."""
+
+    outer_parentheses_optional: bool
+
+    def __new__(cls, value: str, *, outer_parentheses_optional: bool = False):
+        rendered = super().__new__(cls, value)
+        rendered.outer_parentheses_optional = outer_parentheses_optional
+        return rendered
+
+
 @dataclass
 class SubstituentItem:
     name: str

src/openclatura/assembly_prefixes.py

@@ -3,7 +3,7 @@
 import re
 
 from .assembly_charge import inferred_ionic_retained_parent, single_charged_replacement_locants
-from .assembly_parts import AssemblyParts, SubstituentItem
+from .assembly_parts import AssemblyParts, RenderedSubstituentName, SubstituentItem
 from .assembly_utils import is_fully_enclosed, needs_hyphen, parse_locant
 from .formatting import is_complex_prefix
 from .nomenclature import RULES
@@ -156,13 +156,13 @@ def format_substituent_prefixes(parts: AssemblyParts, spiro_subs) -> str:
         mult = (multipliers.complex_(count) if is_complex else multipliers.basic(count)) if count > 1 else ""
         loc_str = substituent_locant_string(parts, locs, len(grouped), spiro_subs)
 
-        name_to_use = name
-        if is_complex and not is_fully_enclosed(name):
+        name_to_use = _omit_optional_outer_parentheses(parts, name, count, loc_str, len(grouped))
+        if is_complex and not is_fully_enclosed(name_to_use):
             if count > 1 or loc_str:
-                name_to_use = f"({name})"
-        elif not loc_str and len(grouped) > 1 and not is_fully_enclosed(name):
+                name_to_use = f"({name_to_use})"
+        elif not loc_str and len(grouped) > 1 and not is_fully_enclosed(name_to_use):
             if name not in ["fluoro", "chloro", "bromo", "iodo"]:
-                name_to_use = f"({name})"
+                name_to_use = f"({name_to_use})"
         prefix_parts.append(f"{loc_str}-{mult}{name_to_use}" if loc_str else f"{mult}{name_to_use}")
 
     prefix_str = prefix_parts[0]
@@ -171,6 +171,28 @@ def format_substituent_prefixes(parts: AssemblyParts, spiro_subs) -> str:
     return prefix_str
 
 
+def _omit_optional_outer_parentheses(
+    parts: AssemblyParts,
+    name: str,
+    count: int,
+    locant_text: str,
+    grouped_count: int,
+) -> str:
+    """Unwrap a directly rendered fragment when its parent boundary is clear."""
+
+    if (
+        parts.is_substituent
+        or count != 1
+        or locant_text
+        or grouped_count != 1
+        or not isinstance(name, RenderedSubstituentName)
+        or not name.outer_parentheses_optional
+        or not is_fully_enclosed(name)
+    ):
+        return name
+    return name[1:-1]
+
+
 def format_replacement_prefixes(parts: AssemblyParts) -> str:
     if not parts.a_prefixes:
         return ""

src/openclatura/component_modifiers.py

@@ -1,82 +1,25 @@
 """Data-driven component modifiers attached after parent numbering."""
 
-from typing import Literal, Protocol, overload
-
 from .assembly_parts import AssemblyParts, NameTokenBinding, SubstituentItem
 from .formatting import strip_outer_parentheses
 from .group_atom_roles import ester_or_peroxy_single_oxygen
 from .locants import parse_locant
 from .molecule import DecisionTrace, Molecule
+from .naming_protocols import RecursiveSubgraphNamer
 from .nomenclature import RULES
 from .perception import PerceivedGroup
 from .subgraph_tools import subgraph_component
 from .substituent_tokens import graph_bound_substituent_tokens
 from .trace_helpers import add_substituent_trace, bond_ids_within, decision_trace_data
 
 
-class BranchNamer(Protocol):
-    """Recursive branch namer with simple and traced/tree return modes."""
-
-    @overload
-    def __call__(
-        self,
-        mol: Molecule,
-        start_idx: int,
-        exclude_atoms: set[int],
-        *,
-        upstream_atom: int | None = None,
-        return_trace: Literal[False] = False,
-        return_tree: Literal[False] = False,
-        decision_trace: DecisionTrace | None = None,
-    ) -> str: ...
-
-    @overload
-    def __call__(
-        self,
-        mol: Molecule,
-        start_idx: int,
-        exclude_atoms: set[int],
-        *,
-        upstream_atom: int | None = None,
-        return_trace: Literal[True],
-        return_tree: Literal[False] = False,
-        decision_trace: DecisionTrace | None = None,
-    ) -> tuple[str, list[dict]]: ...
-
-    @overload
-    def __call__(
-        self,
-        mol: Molecule,
-        start_idx: int,
-        exclude_atoms: set[int],
-        *,
-        upstream_atom: int | None = None,
-        return_trace: Literal[True],
-        return_tree: Literal[True],
-        decision_trace: DecisionTrace | None = None,
-    ) -> tuple[str, list[dict], dict | None]: ...
-
-    @overload
-    def __call__(
-        self,
-        mol: Molecule,
-        start_idx: int,
-        exclude_atoms: set[int],
-        *,
-        upstream_atom: int | None = None,
-        return_trace: Literal[False] = False,
-        return_tree: Literal[True],
-        decision_trace: DecisionTrace | None = None,
-    ) -> tuple[str, dict | None]: ...
-
-
 def add_component_front_modifiers(
     mol: Molecule,
     parts: AssemblyParts,
     perceived_groups: list[PerceivedGroup],
     principal_key: str | None,
     sub_exclude: set[int],
-    branch_namer: BranchNamer,
+    branch_namer: RecursiveSubgraphNamer,
 ) -> None:
     """Add ester/sulfonate front modifiers such as the alcohol component name."""
 
@@ -121,7 +64,7 @@ def add_component_n_substituents(
     numbered_path: list[int],
     get_loc,
     sub_exclude: set[int],
-    branch_namer: BranchNamer,
+    branch_namer: RecursiveSubgraphNamer,
 ) -> None:
     """Add N-substituent prefixes and N/N' locants for principal groups."""
 
@@ -240,7 +183,7 @@ def _nitrogen_substituent_name(
     nitrogen: int,
     substituent: int,
     sub_exclude: set[int],
-    branch_namer: BranchNamer,
+    branch_namer: RecursiveSubgraphNamer,
     decision_trace: DecisionTrace | None = None,
 ) -> tuple[str, list, dict | None]:
     """Render graph-bound N-substituents on principal nitrogen groups."""

src/openclatura/component_namer.py

@@ -1,7 +1,6 @@
 """Connected-component naming pipeline."""
 
 from collections.abc import Callable
-from typing import Literal, Protocol, overload
 
 from .assembly_parts import AssemblyParts, NameAtomBinding, SubstituentItem
 from .chains import find_all_carbon_paths, find_ring_systems, get_cyclic_atoms
@@ -17,6 +16,7 @@
 from .name_bindings import binding_trace_data, refresh_name_atom_bindings
 from .naming_audit import UnnamedAtomError, assert_component_fully_named
 from .naming_context import ComponentNamingState, NamingIntent
+from .naming_protocols import RecursiveSubgraphNamer
 from .parent_pipeline import build_parent_assembly_plan, resolve_retained_parent
 from .parent_selection import select_principal_parent
 from .principal_groups import (
@@ -48,68 +48,12 @@
     assembly_substituent_tree,
     assembly_trace_segments,
     bond_ids_within,
+    build_shortcut_tree_node,
     decision_trace_data,
     functional_group_trace_data,
     trace_decision,
 )
 
-
-class SubgraphNamer(Protocol):
-    """Recursive subgraph namer with simple and traced/tree return modes."""
-
-    @overload
-    def __call__(
-        self,
-        mol: Molecule,
-        start_idx: int,
-        exclude_atoms: set[int],
-        *,
-        upstream_atom: int | None = None,
-        return_trace: Literal[False] = False,
-        return_tree: Literal[False] = False,
-        decision_trace: DecisionTrace | None = None,
-    ) -> str: ...
-
-    @overload
-    def __call__(
-        self,
-        mol: Molecule,
-        start_idx: int,
-        exclude_atoms: set[int],
-        *,
-        upstream_atom: int | None = None,
-        return_trace: Literal[True],
-        return_tree: Literal[False] = False,
-        decision_trace: DecisionTrace | None = None,
-    ) -> tuple[str, list[dict]]: ...
-
-    @overload
-    def __call__(
-        self,
-        mol: Molecule,
-        start_idx: int,
-        exclude_atoms: set[int],
-        *,
-        upstream_atom: int | None = None,
-        return_trace: Literal[True],
-        return_tree: Literal[True],
-        decision_trace: DecisionTrace | None = None,
-    ) -> tuple[str, list[dict], dict | None]: ...
-
-    @overload
-    def __call__(
-        self,
-        mol: Molecule,
-        start_idx: int,
-        exclude_atoms: set[int],
-        *,
-        upstream_atom: int | None = None,
-        return_trace: Literal[False] = False,
-        return_tree: Literal[True],
-        decision_trace: DecisionTrace | None = None,
-    ) -> tuple[str, dict | None]: ...
-
-
 SpiroSubgraphNamer = Callable[[Molecule, int, set[int]], SpiroAssembly]
 ParentAssembler = Callable[..., str]
 
@@ -133,7 +77,7 @@ def collect_component_branch_substituents(
     base_exclude: set[int],
     sub_exclude: set[int],
     *,
-    name_subgraph: SubgraphNamer,
+    name_subgraph: RecursiveSubgraphNamer,
     name_spiro_subgraph: SpiroSubgraphNamer,
     emit_metadata: bool = True,
 ) -> None:
@@ -298,7 +242,7 @@ def name_component(
     return_trace: bool = False,
     return_tree: bool = False,
     decision_trace: DecisionTrace | None = None,
-    name_subgraph: SubgraphNamer,
+    name_subgraph: RecursiveSubgraphNamer,
     name_spiro_subgraph: SpiroSubgraphNamer,
     assemble_parent_name: ParentAssembler,
     token_debug: bool = False,
@@ -332,11 +276,11 @@ def name_component(
             },
         )
         if return_trace and return_tree:
-            return name, [], _shortcut_tree(name, component_atoms, bindings, token_spans)
+            return name, [], _component_shortcut_tree(name, component_atoms, bindings, token_spans)
         if return_trace:
             return name, []
         if return_tree:
-            return name, _shortcut_tree(name, component_atoms, bindings, token_spans)
+            return name, _component_shortcut_tree(name, component_atoms, bindings, token_spans)
         return name
 
     def name_component_again(next_mol: Molecule, next_atoms: set[int], is_substituent: bool = False):
@@ -376,11 +320,11 @@ def name_component_again(next_mol: Molecule, next_atoms: set[int], is_substituen
             },
         )
         if return_trace and return_tree:
-            return name, [], _shortcut_tree(name, component_atoms, bindings, token_spans)
+            return name, [], _component_shortcut_tree(name, component_atoms, bindings, token_spans)
         if return_trace:
             return name, []
         if return_tree:
-            return name, _shortcut_tree(name, component_atoms, bindings, token_spans)
+            return name, _component_shortcut_tree(name, component_atoms, bindings, token_spans)
         return name
 
     state = ComponentNamingState(component_atoms=set(component_atoms), is_substituent=is_substituent)
@@ -431,11 +375,11 @@ def name_component_again(next_mol: Molecule, next_atoms: set[int], is_substituen
             },
         )
         if return_trace and return_tree:
-            return name, [], _shortcut_tree(name, state.component_atoms, bindings, token_spans)
+            return name, [], _component_shortcut_tree(name, state.component_atoms, bindings, token_spans)
         if return_trace:
             return name, []
         if return_tree:
-            return name, _shortcut_tree(name, state.component_atoms, bindings, token_spans)
+            return name, _component_shortcut_tree(name, state.component_atoms, bindings, token_spans)
         return name
 
     state.exclude_atoms = set(mol.atoms.keys()) - state.component_atoms
@@ -654,21 +598,15 @@ def name_component_again(next_mol: Molecule, next_atoms: set[int], is_substituen
     return name
 
 
-def _shortcut_tree(name: str, component_atoms: set[int], bindings: list[dict], token_spans: list[dict]) -> dict:
+def _component_shortcut_tree(
+    name: str, component_atoms: set[int], bindings: list[dict], token_spans: list[dict]
+) -> dict:
     """Return a minimal component tree for shortcut component names."""
 
-    return {
-        "kind": "component",
-        "name": name,
-        "atoms": sorted(component_atoms),
-        "bonds": [],
-        "parent": None,
-        "principal_group": None,
-        "substituents": [],
-        "replacement_prefixes": [],
-        "unsaturations": [],
-        "trace_segments": [],
-        "nested_decisions": [],
-        "name_atom_bindings": bindings,
-        "name_token_spans": token_spans,
-    }
+    return build_shortcut_tree_node(
+        kind="component",
+        name=name,
+        atom_ids=component_atoms,
+        name_atom_bindings=bindings,
+        name_token_spans=token_spans,
+    )