ANTsX / ANTsPy

A fast medical imaging analysis library in Python with algorithms for registration, segmentation, and more.
https://antspyx.readthedocs.io
Apache License 2.0
638 stars 162 forks source link

Added ANTsSerializer to replace previous `_int_antsProcessArguments` and `get_pointer_string` #435

Closed dipterix closed 1 year ago

dipterix commented 1 year ago

Added ANTsSerializer in process_args.py to fix https://github.com/ANTsX/ANTsPy/issues/429 on Windows

It handles image pointers differently on windows: ANTsImages are saved to temporary file instead of returning memory pointers. However, the temporary files need to be cleaned after running command.

This class requires with syntax:

with utils.ANTsSerializer() as serializer:
    out = serializer.get_pointer_string(image)
    ...
    params = int_antsProcessArguments(args)
    libfn(params)

I can't compile ANTsPy from source on my machine (neither osx nor windows). Calling for help if anyone can test the code : )

dipterix commented 1 year ago

@stnava @tfmoraes It'll be great if you could help, also feel free to edit PR if errors out, or to close the PR if you don't like the fix : )

cookpa commented 1 year ago

Another Windows user posted an issue (#437) so I did some digging. I think the issue might be that the serialization on windows produces "[address]", whereas on other platforms it is "0x[address]", where "address" is 16 hex digits for a 64-bit address (it would be good to not hard code this requirement!)

There's two possible solutions

  1. Modify the proposed serializer to reformat the pointers. This assumes that an address having string form "0x[address]" would cast correctly to a pointer on Windows.

  2. Modify ANTs C++ such that "[address]" is treated as a pointer, much as "0x[address]" is - I think I can do this.

EDIT: Windows pointers are just "[address]", I was counting incorrectly. Hoping to fix in #438

stnava commented 1 year ago

Note: we use this same trick in ANTsR - so I would prefer the 2nd approach if it is possible.

cookpa commented 1 year ago

Ok I'll try that!

cookpa commented 1 year ago

I think this is no longer needed (#442), but thanks @dipterix and others for helping to orient me to where the fixes needed to go