search

freesurfer / surfa

Python utilities for medical image and surface-based analysis
24 stars 8 forks source link

introduce class DeformField to support mgzwarp #21

Closed yhuang43 closed 7 months ago

yhuang43 commented 7 months ago

methods implemented:

  1. constructor The constructor can be invoked in two ways: a. invoked with parameters passed b. invoked without any parameters, load(mgzwarp) needs to be called after the object is created
  2. load(filename): Read input mgz warp file, set up deformation field, source/target geometry 3. save(filename): Output deformation field as mgz warp volume 4. change_space(newformat): Return the deformation field as the new data format 5. apply(image): Apply deformation field on given image using Cython interpolation in image/interp.pyx
yhuang43 commented 7 months ago

Thanks Jackson! I actually thought about moving those two utility functions to geometry.py. I don't see other codes use that functionality now. So maybe we can move them when they are needed in other places?

jnolan14 commented 7 months ago

Yeah, I can't think of anything specific off hand where that would be used. Having essentially the same information represented as two different data structures makes me think there would be some utility having some way to easily convert between them, but like you said, we can just make that change when there's a need arises.

yhuang43 commented 7 months ago

I think you are right. Even though there are no other usages for the functionality in the codes now, we can still expose the functions if the users decide to use them. What do you think?

jnolan14 commented 7 months ago

That sounds good to me!

avnishks commented 7 months ago

Looks good to me!

yhuang43 commented 7 months ago

I moved these two utility functions to transform/geometry.py image_geometry2volgeom_dict() (create volgeom dict from an ImageGeometry object) volgeom_dict2image_geometry() (create ImageGeometry object from volgeom dict)

If it looks ok, can you merge it? It doesn't look like I have the permisson to merge.

jnolan14 commented 7 months ago

We may want to remove the prefixed '' from the function names if we intend for them to be accessed by a user. By convention in python functions/methods with a leading '' are intended to be used internally and not really touched by the user, however there's nothing that prevents you from calling those functions. It's just a convention.

PEP standards aren't really my strong suit, Avnish do you have any thoughts on whether it's worth changing the names?

yhuang43 commented 7 months ago

Do you mean the underscore "_" prefix? I removed them in second check in.

jnolan14 commented 7 months ago

That's my mistake, I clicked on the first commit. Looks good to me, merging!