From 7ba0e1fe343d8344f331fb830cd36b2ab3ee02e4 Mon Sep 17 00:00:00 2001 From: Tristan Brennan Date: Sun, 16 Aug 2026 09:21:34 +0900 Subject: [PATCH] Fix TypeError in SubjectRegistration.approxHeadCenter Averaged the LPA/RPA Fiducial objects instead of their planned coordinates, raising TypeError whenever both fiducials were planned. Also guard the property's sole caller (SubjectRegistrationPanel camera alignment on fiducial row selection), which subtracted the property's documented None return from an ndarray whenever LPA/RPA were absent or unplanned, e.g. after planning a NAS fiducial first. The camera now falls back to just resetting on the focal point in that case. --- .../ViewPanels/SubjectRegistrationPanel.py | 6 ++-- .../Navigator/Model/SubjectRegistration.py | 2 +- .../test_subjectRegistrationCalcs.py | 29 +++++++++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 tests/test_Model/test_subjectRegistrationCalcs.py diff --git a/NaviNIBS/Navigator/GUI/ViewPanels/SubjectRegistrationPanel.py b/NaviNIBS/Navigator/GUI/ViewPanels/SubjectRegistrationPanel.py index e888a1d..583b504 100644 --- a/NaviNIBS/Navigator/GUI/ViewPanels/SubjectRegistrationPanel.py +++ b/NaviNIBS/Navigator/GUI/ViewPanels/SubjectRegistrationPanel.py @@ -759,8 +759,10 @@ def _onFidTblCurrentCellChanged(self, currentRow: int, currentCol: int, previous lookAt = None if lookAt is not None: self._plotter.camera.focal_point = lookAt - vec = lookAt - subReg.approxHeadCenter - self._plotter.camera.position = lookAt + vec*10 + headCenter = subReg.approxHeadCenter + if headCenter is not None: + vec = lookAt - headCenter + self._plotter.camera.position = lookAt + vec*10 self._plotter.reset_camera() def _getSelectedFiducialKeys(self): diff --git a/NaviNIBS/Navigator/Model/SubjectRegistration.py b/NaviNIBS/Navigator/Model/SubjectRegistration.py index 766d57a..e93490b 100644 --- a/NaviNIBS/Navigator/Model/SubjectRegistration.py +++ b/NaviNIBS/Navigator/Model/SubjectRegistration.py @@ -509,7 +509,7 @@ def approxHeadCenter(self) -> tp.Optional[np.ndarray]: lpa = self.fiducials.get('LPA', None) rpa = self.fiducials.get('RPA', None) if lpa is not None and rpa is not None and lpa.plannedCoord is not None and rpa.plannedCoord is not None: - center = (lpa + rpa) / 2 + center = (lpa.plannedCoord + rpa.plannedCoord) / 2 else: logger.warning('Insufficient information for determining approximate header center') # TODO: implement more general method of estimating center, e.g. more variety of LPA/RPA naming, name-agnostic averaging, etc. diff --git a/tests/test_Model/test_subjectRegistrationCalcs.py b/tests/test_Model/test_subjectRegistrationCalcs.py new file mode 100644 index 0000000..aa1c5bf --- /dev/null +++ b/tests/test_Model/test_subjectRegistrationCalcs.py @@ -0,0 +1,29 @@ +""" +Regression tests for SubjectRegistration derived values. +""" + +import numpy as np + +from NaviNIBS.Navigator.Model.SubjectRegistration import SubjectRegistration, Fiducial + + +def test_approxHeadCenterAveragesPlannedCoords(): + """ + Previously averaged the Fiducial objects themselves instead of their + planned coordinates, raising TypeError whenever LPA and RPA were planned. + """ + sr = SubjectRegistration() + sr.fiducials.addItem(Fiducial(key='LPA', plannedCoord=np.array([-80., 0., -40.]))) + sr.fiducials.addItem(Fiducial(key='RPA', plannedCoord=np.array([80., 0., -40.]))) + + center = sr.approxHeadCenter + assert center is not None + np.testing.assert_allclose(center, np.array([0., 0., -40.])) + + +def test_approxHeadCenterMissingFiducials(): + sr = SubjectRegistration() + assert sr.approxHeadCenter is None + + sr.fiducials.addItem(Fiducial(key='LPA', plannedCoord=np.array([-80., 0., -40.]))) + assert sr.approxHeadCenter is None