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
608 stars 161 forks source link

tidy "NO CLUE WHAT THIS DOES OR WHY IT'S NEEDED" #515

Closed LeeReid1 closed 8 months ago

LeeReid1 commented 8 months ago

While trying to debug something of my own, I found your apply transforms code contains the following

            # NO CLUE WHAT THIS DOES OR WHY IT'S NEEDED
            for jj in range(len(myargs)):
                if myargs[jj] is not None:
                    if myargs[jj] == '-':
                        myargs2 = [None]*(len(myargs)-1)
                        myargs2[:(jj-1)] = myargs[:(jj-1)]
                        myargs2[jj:(len(myargs)-1)] = myargs[(jj+1):(len(myargs))]
                        myargs = myargs2

It seems to be trimming hyphens from the start of each argument..?

Something like this might be simpler:

myargs = [None if s is None else s.lstrip('-') for s in myargs]

Though I note the first will remove only one hyphen and mine will remove all. Not sure what was intended.

Apologies that I have no time to test or do a pull request. Hope that helps though

cookpa commented 8 months ago

Thanks for flagging this. I think it's wanting to remove instances of '-' from the myargs list, but in a very weird way that won't actually do anything.

If myargs contains '-', it will cause an error because it shortens myargs within a loop while iterating over the original index length.

If myargs does not contain '-', it won't do anything.

@stnava @ntustison am I missing a use case where stripping '-' from the args list might be needed? Is there a reason to fix this rather than just remove it entirely?

ntustison commented 8 months ago

Not that I'm aware of. I have no idea why this snippet is there.