Skip to content

Commit d8ff897

Browse files
committed
fix: _prefer_spiro_backbone_components() now maps shared counts for every RingSystem.paths entry, not just paths[0].
Removed the redundant ring_system_by_path lookup. Added _spiro_backbone_rank_key() so the local backbone ranking tuple is defined once.
1 parent d74d999 commit d8ff897

4 files changed

Lines changed: 113 additions & 34 deletions

File tree

src/bluenamer/parent_selection.py

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -376,11 +376,11 @@ def _prefer_spiro_backbone_components(
376376
if len(ring_systems) < 2:
377377
return candidates
378378

379-
ring_system_by_path = {tuple(system.paths[0]): system for system in ring_systems}
380379
shared_counts: dict[tuple[int, ...], int] = {}
381380
for system in ring_systems:
382-
path_key = tuple(system.paths[0])
383-
shared_counts[path_key] = sum(1 for other in ring_systems if other is not system and system.atoms & other.atoms)
381+
shared_count = sum(1 for other in ring_systems if other is not system and system.atoms & other.atoms)
382+
for path in system.paths:
383+
shared_counts[tuple(path)] = shared_count
384384

385385
max_shared = max(shared_counts.values(), default=0)
386386
if max_shared == 0:
@@ -402,39 +402,43 @@ def _prefer_spiro_backbone_components(
402402
)
403403
== best_principal_key
404404
]
405-
eligible_ring_components = [
406-
candidate
407-
for candidate in eligible
408-
if candidate.is_ring
409-
and shared_counts.get(tuple(candidate.path), 0) > 0
410-
and tuple(candidate.path) in ring_system_by_path
411-
]
405+
eligible_ring_components = []
406+
for candidate in eligible:
407+
if not candidate.is_ring:
408+
continue
409+
path_key = tuple(candidate.path)
410+
if shared_counts.get(path_key, 0) <= 0:
411+
continue
412+
eligible_ring_components.append(candidate)
412413
if len(eligible_ring_components) < 2:
413414
return candidates
414415

415-
best_backbone_key = min(
416-
(
417-
-shared_counts[tuple(candidate.path)],
418-
-candidate.seniority_profile.ring_count,
419-
-candidate.seniority_profile.parent_atom_count,
420-
candidate.seniority_profile.path_tiebreak,
421-
)
416+
backbone_keys = {
417+
tuple(candidate.path): _spiro_backbone_rank_key(candidate, shared_counts)
422418
for candidate in eligible_ring_components
423-
)
424-
backbone = [
419+
}
420+
best_backbone_key = min(backbone_keys.values())
421+
backbone_paths = {path for path, key in backbone_keys.items() if key == best_backbone_key}
422+
if not backbone_paths:
423+
return candidates
424+
competing_paths = set(backbone_keys)
425+
return [
425426
candidate
426-
for candidate in eligible_ring_components
427-
if (
428-
-shared_counts[tuple(candidate.path)],
429-
-candidate.seniority_profile.ring_count,
430-
-candidate.seniority_profile.parent_atom_count,
431-
candidate.seniority_profile.path_tiebreak,
432-
)
433-
== best_backbone_key
427+
for candidate in candidates
428+
if tuple(candidate.path) not in competing_paths or tuple(candidate.path) in backbone_paths
434429
]
435-
if not backbone:
436-
return candidates
437-
return backbone
430+
431+
432+
def _spiro_backbone_rank_key(candidate: ParentCandidate, shared_counts: dict[tuple[int, ...], int]) -> tuple:
433+
"""Return the local rank key for competing spiro-connected ring components."""
434+
435+
path_key = tuple(candidate.path)
436+
return (
437+
-shared_counts[path_key],
438+
-candidate.seniority_profile.ring_count,
439+
-candidate.seniority_profile.parent_atom_count,
440+
candidate.seniority_profile.path_tiebreak,
441+
)
438442

439443

440444
def _ring_count_for_system(ring_system: RingSystem) -> int:

src/bluenamer/tests/test_analysis.py

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
format_spiro_core,
3434
split_spiro_substituents,
3535
)
36-
from bluenamer.chains import find_all_carbon_paths, find_ring_systems
36+
from bluenamer.chains import RingSystem, find_all_carbon_paths, find_ring_systems
3737
from bluenamer.charge_pair_roles import charge_pair_roles, unsupported_charge_pair_roles
3838
from bluenamer.formatting import ensure_stereo_descriptor_boundary, format_counted_prefixes
3939
from bluenamer.functional_groups import (
@@ -112,6 +112,7 @@
112112
ParentCandidate,
113113
ParentSelection,
114114
ParentSeniorityProfile,
115+
_prefer_spiro_backbone_components,
115116
select_principal_parent,
116117
)
117118
from bluenamer.perception import PerceivedGroup, perceive_groups
@@ -3276,6 +3277,81 @@ def test_parent_selection_criteria_are_data_ordered():
32763277
assert profile.score_tuple() == (-1, -1, (7,), 0, (), (-2, 0, (0, 0, 0, 0, 0, 0)), 0, 0, (0, 1))
32773278

32783279

3280+
def test_senior_element_vector_orders_ring_and_chain_parent_profiles():
3281+
def profile(*, ring_parent: bool, senior_element_vector: tuple[int, ...]) -> ParentSeniorityProfile:
3282+
return ParentSeniorityProfile(
3283+
principal_group_count=0,
3284+
contains_principal_group=False,
3285+
senior_element_vector=senior_element_vector,
3286+
polycycle_parent=False,
3287+
bicycle_parent=False,
3288+
spiro_parent=False,
3289+
ring_parent=ring_parent,
3290+
ring_count=1 if ring_parent else 0,
3291+
parent_atom_count=4,
3292+
heteroatom_count=1,
3293+
senior_heteroatom_vector=(),
3294+
senior_heteroatom_count_vector=(0, 0, 0, 0, 0, 0),
3295+
multiple_bond_count=0,
3296+
double_bond_count=0,
3297+
path_tiebreak=(0, 1, 2, 3),
3298+
)
3299+
3300+
n_ring = profile(ring_parent=True, senior_element_vector=(1, 7))
3301+
o_ring = profile(ring_parent=True, senior_element_vector=(5, 7))
3302+
n_chain = profile(ring_parent=False, senior_element_vector=(1, 7))
3303+
o_chain = profile(ring_parent=False, senior_element_vector=(5, 7))
3304+
3305+
assert n_ring.score_tuple() < o_ring.score_tuple()
3306+
assert n_chain.score_tuple() < o_chain.score_tuple()
3307+
3308+
3309+
def test_spiro_backbone_filter_keeps_noncompeting_candidates_and_matches_all_ring_paths():
3310+
backbone = ParentCandidate.build(
3311+
[3, 2, 1, 0],
3312+
is_ring=True,
3313+
is_bicycle=False,
3314+
is_spiro=True,
3315+
is_polycycle=False,
3316+
xyz=(0, 0, 0),
3317+
principal_groups_count=0,
3318+
mol=None,
3319+
ring_count=2,
3320+
)
3321+
side_ring = ParentCandidate.build(
3322+
[3, 4, 5],
3323+
is_ring=True,
3324+
is_bicycle=False,
3325+
is_spiro=False,
3326+
is_polycycle=False,
3327+
xyz=(0, 0, 0),
3328+
principal_groups_count=0,
3329+
mol=None,
3330+
ring_count=1,
3331+
)
3332+
unrelated_chain = ParentCandidate.build(
3333+
[10, 11, 12],
3334+
is_ring=False,
3335+
is_bicycle=False,
3336+
is_spiro=False,
3337+
is_polycycle=False,
3338+
xyz=(0, 0, 0),
3339+
principal_groups_count=0,
3340+
mol=None,
3341+
)
3342+
ring_systems = [
3343+
RingSystem(atoms={0, 1, 2, 3}, is_spiro=True, paths=[[0, 1, 2, 3], [3, 2, 1, 0]]),
3344+
RingSystem(atoms={3, 4, 5}, paths=[[3, 4, 5]]),
3345+
]
3346+
3347+
filtered = _prefer_spiro_backbone_components([side_ring, unrelated_chain, backbone], ring_systems)
3348+
3349+
assert backbone in filtered
3350+
assert unrelated_chain in filtered
3351+
assert side_ring not in filtered
3352+
assert [candidate.path for candidate in filtered] == [[10, 11, 12], [3, 2, 1, 0]]
3353+
3354+
32793355
def test_parent_seniority_profile_exposes_brief_guide_extension_fields():
32803356
mol = read_smiles("NCCO")
32813357
candidate = ParentCandidate.build(

tests/integration/test_corpus_golden.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,5 @@ def test_corpus_names_match_golden(goldens, capsys):
5858
print(f" expected: {want}")
5959
print(f" got: {got}")
6060
pytest.fail(
61-
f"{len(mismatches)} corpus entries name differently on rdkit "
62-
f"{rdkit.__version__} than the committed goldens"
61+
f"{len(mismatches)} corpus entries name differently on rdkit {rdkit.__version__} than the committed goldens"
6362
)

tests/integration/test_diverse_corpus.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ def test_corpus_naming_pass_rate(corpus):
8282
# Surface the offenders so the failure is actionable.
8383
sample = "\n".join(f" - {s} [{c}]" for s, c in unnamed[:10])
8484
pytest.fail(
85-
f"Naming pass rate {rate:.2%} below floor {NAME_PASS_RATE_FLOOR:.2%}.\n" f"First unnamed entries:\n{sample}"
85+
f"Naming pass rate {rate:.2%} below floor {NAME_PASS_RATE_FLOOR:.2%}.\nFirst unnamed entries:\n{sample}"
8686
)
8787

8888

0 commit comments

Comments
 (0)