Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Neatened things up to be more reader-friendly #766

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DeanTM
Copy link

@DeanTM DeanTM commented Jan 10, 2025

Hey fellas. This isn't a proper pull request because I haven't done any tests (I'm not actually sure what to do re standards for this library?); rather, I've just I've just made the registrations.py file a little more reader-friendly.

Motivation

I'm trying to use ANTsPy to register light-sheet microscopy data of larval zebrafish brains, but am having limited success. I'm having to dig through the code to understand what arguments go where, which I find easier to do after renaming and reorganising a few things.

My hope is that now, if a person wants to understand a specific type of transform, they can read how the arguments args are constructed and work backwards from there.

What I've tweaked

  • by comparing this Anatomy of an antsRegistration call post with the type_of_transform="antsRegistrationSyN[s]" case I was able to recover the full length names of some of the arguments; I used this to rename some arguments. I've also renamed some of the shorter-named variables.
  • some of the variables had redundant or tautologous expressions, like if [Boolean Variable] == True, so I simplified those.
  • I aligned the arguments in the list format so that they're easier to read (though possibly less PEP8-friendly).
  • I've also noticed that the type of transform "SyNLessAggro" seems to do exactly the same as "SyNAggro", so I've added a comment to that in the function description.

@ntustison
Copy link
Member

Thanks @DeanTM . I did a glance over and it looks pretty clean to me but I won't be able to do any testing until next week. If @cookpa and @stnava haven't approved of it by then, I'll work on merging it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants