Skip to content

antsRegistrationSyNQuick (continuation of #1392) #2347

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

Closed
wants to merge 485 commits into from

Conversation

djarecka
Copy link
Collaborator

continuing #1392

not sure where to find a good description of RegistrationSyNQuick, here I can find only a few usage examples.

@oesteban oesteban added this to the 0.14.1 milestone Jan 7, 2018
@mgxd
Copy link
Member

mgxd commented Jan 10, 2018

@djarecka anymore additions or is this ready?

@@ -1,157 +0,0 @@
# AUTO-GENERATED by tools/checkspecs.py - DO NOT EDIT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still wanna keep this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't remove it manually. I mentioned in #2349 that since we have a non-auto test, this test has been removed by make specs. What happen when you run make specs?

oesteban and others added 26 commits January 17, 2018 23:23
small changes in  FSSourceInputSpec (fixes nipy#2116)
* 'master' of github.com:nipy/nipype:
  fix error type
  remove install_aliases
  sty
  fix tests
  refactoring tests
  update changes
  [MAINT] Cleanup engine/base
* origin/master: (139 commits)
  fix error type
  remove install_aliases
  sty
  fix tests
  refactoring tests
  update changes
  [MAINT] Cleanup engine/base
  STY: Add newline
  pep
  formatting
  Revert "after make specs"
  address review and add changelog
  MAINT: Adjust test names to placate make specs
  Fix 3dFWHMx outputs
  fix: composemultitransform doctest
  sty: fix examples
  sty: fix styles in updated interfaces
  sty: cleanup tools folder
  sty: add yapf to extras installs
  fix: specs
  ...
@satra satra added this to the 1.0.1 milestone Feb 15, 2018
desc="A prefix that is prepended to all output files")

# todo ANTSCommandInputSpec already has this, but I can't figure out how to set it without defining it again
num_threads = traits.Int(default_value=1, desc='Number of threads (default = 1)', argstr='-n %d')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should check if the script checks for the environment variables that is set by the base class instead of this argument. if so the underlying commands in the script will use it.

but since this script has an explicit flag for threads, you should over-ride the base class.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@satra - don't understand how this should be changed. Should num_threads be in RegistrationSynQuickInputSpec (as it is now), or over-ride somewhere else?

In this script the default value is -n 1, so that's why probably the author of changes used `default_value=1.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be in:

RegistrationSynQuickInputSpec (as it is now)

since the script explicitly overrides this: https://github.com/ANTsX/ANTs/blob/master/Scripts/antsRegistrationSyNQuick.sh#L388


class RegistrationSynQuick(ANTSCommand):
"""
Examples
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can just use some of the ants registration examples here in much simplified form.

@djarecka
Copy link
Collaborator Author

something went wrong, will open a new pr

@djarecka djarecka closed this Feb 20, 2018
effigies added a commit that referenced this pull request Feb 23, 2018
antsRegistrationSyNQuick (continuation of #1392 and #2347)
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.