BUG: Fix 4DCT vrg pipeline not properly centering sequences

BrownBiomechanics / SlicerAutoscoperM

This 3D Slicer extension enables users to perform image registration.

https://autoscoperm.slicer.org

MIT License

1 stars 4 forks source link

BUG: Fix 4DCT vrg pipeline not properly centering sequences #106

Closed NicerNewerCar closed 2 weeks ago

NicerNewerCar commented 5 months ago

Regression introduced in e036c2249dc564c2cca03914837248dae6d06666

Closes #105

Also resolves this comment https://github.com/BrownBiomechanics/SlicerAutoscoperM/issues/102#issuecomment-2180735267

NicerNewerCar commented 4 months ago

[x] This for loop needs to be changed to for i in range(1,numFrames) so we don't repeat the first item in the sequence. https://github.com/BrownBiomechanics/SlicerAutoscoperM/blob/1e87e54d110b982335ff8493040351de083000d1/AutoscoperM/AutoscoperM.py#L570-L584

jcfr commented 4 months ago

We are getting close :pray:

Few nitpicks:

move fix into its own commit
add the performance change into its own commit
move the removal of the threadexecutor into its own pull request

NicerNewerCar commented 4 months ago

Ideally, we should also look into adding support to call GetCenterPositionRAS from Python, this would simplify the logic.

This doesn't seem to be exposed in the Python API:

>>> x.GetCenterPositionRAS()
Traceback (most recent call last):
  File "<console>", line 1, in <module>
AttributeError: 'MRMLCorePython.vtkMRMLScalarVolumeNode' object has no attribute 'GetCenterPositionRAS'

It also seems to be a protected function for the vtkMRMLVolumeNode. Was this exposed somewhere else?

amymmorton commented 2 weeks ago

Suggest with the depreciation of vrgs- also close this