SlicerIGT / SlicerMarkupsToModel

3D Slicer extension to create tube or closed surface model from markup fiducials
BSD 3-Clause "New" or "Revised" License
11 stars 10 forks source link

ENH: Add MRMLNode macros to vtkMRMLMarkupsToModelNode #28

Closed Sunderlandkyl closed 6 years ago

Sunderlandkyl commented 6 years ago

Added functionality to Copy and PrintSelf methods. Changed WriteXML and ReadXMLAttributes to use MRML node macros.

Sunderlandkyl commented 6 years ago

@lassoan There are a couple of logical differences between the two versions. In ReadXMLAttributes, there is a number of attributes that used to have some bounds checking on them and error/warning messages.

Ex. ModelType, KochanekBias, etc.

How important is this bounds checking? Should I restore it for these specific attributes?

lassoan commented 6 years ago

How important is this bounds checking?

It is good to have them. Probably we can implement them by simply replacing vtkSetClampMacro by vtkSetClampMacro.

It would be also nice to handle legacy attribute names - see how it is done in vtkMRMLCameraNode::ReadXMLAttributes.

lassoan commented 6 years ago

@Sunderlandkyl I'm ready to merge this if you implement what is discussed above (use vtkSetClampMacro and handle legacy attribute names). Thanks!

Sunderlandkyl commented 6 years ago

vtkSetClampMacro added.

I already had:

vtkMRMLReadXMLEnumMacro(InterpolationType, CurveType);

as well as:

vtkMRMLReadXMLEnumMacro(CurveType, CurveType);

which should handle the InterpolationType legacy attribute name for CurveType.

Are there any other legacy attributes that need to be handled?

lassoan commented 6 years ago

Thank you!