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

Remove ANTSPATH #1574

Merged
merged 1 commit into from
Jul 26, 2023
Merged

Remove ANTSPATH #1574

merged 1 commit into from
Jul 26, 2023

Conversation

ghisvail
Copy link
Contributor

Closes #1573

@ghisvail
Copy link
Contributor Author

ghisvail commented Jul 25, 2023

Looks like I am missing a few files (32 vs 34 in the search).

Got all of them.

To be noted, Scripts/multi_template_script.sh requires ANTSPATH as its first positional argument. I left the CLI as is to not break existing workflows, it just does not do anything with it anymore.

@ghisvail ghisvail force-pushed the rm-antspath branch 2 times, most recently from def34a9 to 7fa9a00 Compare July 25, 2023 15:32
@cookpa
Copy link
Member

cookpa commented Jul 25, 2023

Most of the scripts still need some work - removing ANTSPATH references is not sufficient because many of the checks for programs will fail.

Eg

-s "${ANTSPATH}ANTS"

can't be replaced with

-s "ANTS"

but rather needs to be -s $(which ANTS).

Is it OK if I push to your branch?

@ghisvail
Copy link
Contributor Author

ghisvail commented Jul 25, 2023 via email

@gdevenyi
Copy link
Contributor

POSIX check for a command is command -v as which may not always be available

@ghisvail
Copy link
Contributor Author

POSIX check for a command is command -v as which may not always be available

PR updated with if ! command -v $CMD &> /dev/null guards as per this SO thread.

if [[ ! -f "${ANTSPATH}/ANTS" ]] ; then
echo "Cannot find the ANTS program. Please \(re\)define \$ANTSPATH in your environment."

if ! command -v "ANTS" &> /dev/null
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 you need the square brackets still

Copy link
Contributor Author

@ghisvail ghisvail Jul 25, 2023

Choose a reason for hiding this comment

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

It works without brackets on my machine. I used the pattern in the accepted answer of the SO thread linked above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

# Make ANTS disappear from PATH.
$ mv $HOME/.local/ants/bin/ANTS{,.bak}

# Run wrapper script.
$ ./Scripts/ants.sh
Cannot find the ANTS program. Please \(re\)define $PATH in your environment.

# Put it back.
$ mv $HOME/.local/ants/bin/ANTS{.bak,}

# Run wrapper script again.
$ ./Scripts/ants.sh
 USAGE ::  
  sh   ants.sh  ImageDimension  fixed.ext  moving.ext  OPTIONAL-OUTPREFIX   OPTIONAL-max-iterations  OPTIONAL-Labels-In-Fixed-Image-Space-To-Deform-To-Moving-

@cookpa
Copy link
Member

cookpa commented Jul 26, 2023

Looking good now @ghisvail

@cookpa cookpa merged commit f3c720e into ANTsX:master Jul 26, 2023
@cookpa
Copy link
Member

cookpa commented Jul 26, 2023

Thanks for doing this @ghisvail

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.

Do we still need ANTSPATH?
3 participants