is.na(mask) - argument is not interpretable as logical

[dorianps]

is.na(mask) - argument is not interpretable as logical

This error is coming up on OSX installations of ANTsR/LINDA/LESYMAP. The error shows when an antsImage is checked if is NA (the default value if the user doesn't specify the argument). There is no problem on linux, so there must be something unusual going on in Mac. Error came up in Travis tests for LESYMAP last night, but also a user had the same problem yesterday while using LINDA, issue here.

I don't have an Apple computer and I have no clue why all of sudden checking if an antsImage variable is NA has become a problem there. Any idea?

[ntustison]

I don't have an answer for you but I had the same issue pop up with ANTsRNet travis builds a couple weeks ago and ended up switching to NULL usage.

[dorianps]

You had the problem on OSX as well, right?

Did you simply switch to is.null and worked?

I wonder whether this is a change of R variable policy for the latest release, I would be surprised if this was just a mac problem.

[ntustison]

It was on all the travis builds which include both linux and osx. The link above points to my fix commit and, yes, that is all I did was change NA --> NULL and "is.na" --> "is.NULL" in the case of an antsImage. That was all it took to correct the issue.

[muschellij2]

These are changes that I made. This is due to the fact that is.na, like on most other objects, should return an image with logical indicators whether that value is missing or not.

This is a change to most of the underlying code with respect to ANTsRCore. If you like to check with the default argument is and use that, you can use the formals or the formalArgs Functions to check it or you can check specific versions of the core code

On Sat, Jul 20, 2019 at 11:46 AM Nick Tustison notifications@github.com wrote:

It was on all the travis builds which include both linux and osx. The link above points to my fix commit and, yes, that is all I did was change NA --> NULL and "is.na" --> "is.NULL" in the case of an antsImage. That was all it took to correct the issue.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/274?email_source=notifications&email_token=AAIGPLWFQOLWZI2VQBLUKZDQAMXNTA5CNFSM4IFPACYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2NQ2OI#issuecomment-513477945, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIGPLWWHQ4V24BUW7C6WE3QAMXNTANCNFSM4IFPACYA .

-- John

[dorianps]

Makes sense the change you made. I don't remember seeing a pull request with these changes though, was there one? Should we perhaps keep a common tracker of changes to ANTsR so we can go back to read changes without scavenging code to understand what happens (I am thinking something like the NEWS file or a wiki page).

P.s. Which commit did the is.na change happen?

[dorianps]

@muschellij2 The problem is not coming from the LINDA pakcage but from abpN4() in ANTsR. That function contains is.na(mask). I ran a quick check and looks like ANTsR uses the NA strategy for default antsImage arguments frequently. I tried fixing abpN4 myself by switching NA to NULL as suggested by @ntustison but then another error appeared for antsRegistration calls (object 'maskopt' not found). I also tried deleting the whole file zzz_finite.R which you had added last month, and besides new warnings, the maskopt error appears again.

9:39 Loading template...
19:39 Skull stripping... (long process) bad det -1 v 1 u -1
 bad det -1 v 1 u -1 new 1
Error in antsRegistration(tem, img, typeofTransform = regtype, initialTransform = initafffn,  : 
  object 'maskopt' not found
In addition: Warning messages:
1: In is.na(mask) : is.na() applied to non-(list or vector) of type 'S4'
2: In is.na(mask) : is.na() applied to non-(list or vector) of type 'S4'
Called from: antsRegistration(tem, img, typeofTransform = regtype, initialTransform = initafffn, 
    mask = temregmask, verbose = verbose)

You know better what changes you made these last couple of months. Can you please apply a fix to make things work at least in the short while. That is, either roll back the changes to allow the LINDA users get going, or edit all the functions that use is.na in ANTsR.

[dorianps]

This is the temporary solution, using a June 13 commit for ANTsRCore:

git clone https://github.com/ANTsX/ANTsRCore.git
cd ANTsRCore
git checkout 25f0f8a469950f3c66ece4c2632bb15d30cea8de
R CMD INSTALL .

[muschellij2]

There was one but it was a bit muddled: https://github.com/ANTsX/ANTsRCore/pull/87/files John

On Sat, Jul 20, 2019 at 6:04 PM dorianps notifications@github.com wrote:

Makes sense the change you made. I don't remember seeing a pull request with these changes though, was there one? Should we perhaps keep a common tracker of changes to ANTsR so we can go back to read changes without scavenging code to understand what happenes (I am thinking something like the NEWS file or a wiki page).

P.s. Which commit did the is.na change happen?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/274?email_source=notifications&email_token=AAIGPLQO7KZGEAB6BXGP7D3QAODXHA5CNFSM4IFPACYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2NWZXI#issuecomment-513502429, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIGPLTFT3X3GYHK5A3MZDTQAODXHANCNFSM4IFPACYA .

[muschellij2]

I think it depends on your version of ANTsRCore.

John

On Sun, Jul 21, 2019 at 7:54 PM dorianps notifications@github.com wrote:

@muschellij2 https://github.com/muschellij2 The problem is not coming from the LINDA pakcage but from abpN4() in ANTsR. That function contains is.na(mask). I ran a quick check and looks like ANTsR uses the NA strategy for default antsImage arguments frequently. I tried fixing abpN4 myself by switching NA to NULL as suggested by @ntustison https://github.com/ntustison but then another error appeared for antsRegistration calls (object 'maskopt' not found). I also tried deleting the whole file zzz_finite.R which you had added last month, and besides new warnings, the maskopt error appears again.

9:39 Loading template... 19:39 Skull stripping... (long process) bad det -1 v 1 u -1 bad det -1 v 1 u -1 new 1 Error in antsRegistration(tem, img, typeofTransform = regtype, initialTransform = initafffn, : object 'maskopt' not found In addition: Warning messages: 1: In is.na(mask) : is.na() applied to non-(list or vector) of type 'S4' 2: In is.na(mask) : is.na() applied to non-(list or vector) of type 'S4' Called from: antsRegistration(tem, img, typeofTransform = regtype, initialTransform = initafffn, mask = temregmask, verbose = verbose)

You know better what changes you made these last couple of months. Can you please apply a fix to make things work at least in the short while. That is, either roll back the changes to allow the LINDA users get going, or edit all the functions that use is.na in ANTsR.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/274?email_source=notifications&email_token=AAIGPLQQZRYOP2MLYPSJIHDQATZMVA5CNFSM4IFPACYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2OOIWI#issuecomment-513598553, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIGPLWTCA2E43SSF3FASY3QATZMVANCNFSM4IFPACYA .

[dorianps]

@muschellij2 Not sure I understand. The current ANTsRCore is not compatible with ANTsR, because certain functions, like abpN4() fail to work. This makes any program that relies on these functions unusable, example being LINDA. Maybe it makes sense to roll back the changes you applied since June and work them in an experimental branch until there are no problems.

[muschellij2]

The current ANTsR works with the ANTsR core. The travis checks show that. I didn’t see you post versions, is it up to date?

On Mon, Jul 22, 2019 at 6:39 PM dorianps notifications@github.com wrote:

@muschellij2 https://github.com/muschellij2 Not sure I understand. The current ANTsRCore is not compatible with ANTsR, because certain functions, like abpN4() fail to work. This makes any program that relies on these functions unusable, example being LINDA. Maybe it makes sense to roll back the changes you applied since June and work them in an experimental branch until there are no problems.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/274?email_source=notifications&email_token=AAIGPLU7S2PN2XY2GHRPZCLQAYZITA5CNFSM4IFPACYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2RLYRI#issuecomment-513981509, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIGPLU54HNZZYIR7KTVXSLQAYZITANCNFSM4IFPACYA .

-- John

[dorianps]

@muschellij2 Travis tests do not test for all combinations of arguments a user can put. Here is a reproducible error:

> img <- antsImageRead(getANTsRData("r16"))
> mask = getMask(img)
> temp = abpN4(img, mask=mask)
Error in if (is.na(mask)) { : argument is not interpretable as logical

[muschellij2]

Agreed- I'm using https://github.com/ANTsX/ANTsR/pull/260, which I assumed ANTsR was using...

On Tue, Jul 23, 2019 at 9:44 PM dorianps notifications@github.com wrote:

@muschellij2 https://github.com/muschellij2 Travis tests do not test for all combinations of arguments a user can put. Here is a reproducible error:

img <- antsImageRead(getANTsRData("r16")) mask = getMask(img) temp = abpN4(img, mask=mask) Error in if (is.na(mask)) { : argument is not interpretable as logical

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/274?email_source=notifications&email_token=AAIGPLVRO4OHX577NEUMWQLQA6XYJA5CNFSM4IFPACYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2U5B6I#issuecomment-514445561, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIGPLTVEX4ZV34JMM47DRTQA6XYJANCNFSM4IFPACYA .

[dorianps]

Tried uninstalling everything (ITKR/ANTsRCore/ANTsR) and install again with just

devtools::install_github('ANTsX/ANTsR')

The error above is the same, meaning that the devtools install method has the same problem (I was using R CMD INSTALL before)

[stnava]

I think that John is suggesting that we need to merge a Pull request

On Wed, Jul 24, 2019 at 2:54 PM dorianps notifications@github.com wrote:

Tried uninstalling everything (ITKR/ANTsRCore/ANTsR) and install again with just

devtools::_install_github('ANTsX/ANTsR')

The error above is the same, meaning that the devtools install method has the same problem (I was using R CMD INSTALL before)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/274?email_source=notifications&email_token=AACPE7VGV6VTCBRNMUMCOHTQBCQNFA5CNFSM4IFPACYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2XI5EI#issuecomment-514756241, or mute the thread https://github.com/notifications/unsubscribe-auth/AACPE7RF5FGEPLYFL32ZKUTQBCQNFANCNFSM4IFPACYA .

--

brian

[dorianps]

Looks like that pull request addresses is.na in many functions, and also passes travis. Is there a reason to hold the merge for now? Knowing, of course, that ANTsRCore and ANTsR are not 100% compatible currently.

[dorianps]

Pinging this issue again. @stnava is there an issue with merging the above pull request? If the merge is not happening soon, maybe ANTsRCore should reverse to an older commit to re-establish the compatibility with ANTsR.

[muschellij2]

I would suggest you use the releases for any stable code you’re working on and not necessarily the master because that is essentially a development branch

On Mon, Jul 29, 2019 at 6:50 PM dorianps notifications@github.com wrote:

Pinging this issue again. @stnava https://github.com/stnava is there an issue with merging the above pull request. If the merge is not happening soon, maybe ANTsRCore should reverse to an older commit to re-establish the compatibility with ANTsR.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/274?email_source=notifications&email_token=AAIGPLRDHP3UXJ7SX3MRQ63QB5X3HA5CNFSM4IFPACYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3CHKAA#issuecomment-516191488, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIGPLX6OZML2Y6HNEQGMA3QB5X3HANCNFSM4IFPACYA .

-- John

[dorianps]

Thanks for merging. Looks like things are working for now.

A small note. Doing is.na(c(mask)) returns still a logical, which can be helpful if we want to overcome the is.na(mask) method of ANTsR which returns and antsImage.

[muschellij2]

Correct but it’s not a single logical indicating if the mask was specified. You can do any(is.na(mask)) or all(is.na(mask)), but that’s not really the same test as before.

On Tue, Jul 30, 2019 at 9:40 PM dorianps notifications@github.com wrote:

Thanks for merging. Looks like things are working for now.

A small note. Doing is.na(c(mask)) returns still a logical, which can be helpful if we want to overcome the is.na method of ANTsR which returns and antsImage.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/274?email_source=notifications&email_token=AAIGPLWG6FM3A2YVSSZBJMTQCDUQVA5CNFSM4IFPACYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3FZKJA#issuecomment-516658468, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIGPLVUDLIYPLXIS3DEAXDQCDUQVANCNFSM4IFPACYA .

-- Best, John

[dorianps]

Doing is.na(c(mask)) produces a single logical. That is the strategy I decided to use to correct my code instead of going for is.null()

[muschellij2]

I agree, but I would not rely on this behavior. c(mask) turns mask into a list of antsImage objects. This is likely to change in the future after some discussions. That is because and antsImage object should IMO act like an R-array when operated on, therefore any OP should act in a similar faction on an image as OP(as.array(image)). And c(as.array(mask)) returns a vector.

Best, John

On Wed, Jul 31, 2019 at 10:40 AM dorianps notifications@github.com wrote:

Doing is.na(c(mask)) produces a single logical. That is the strategy I decided to use to correct my code instead of going for is.null()

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsR/issues/274?email_source=notifications&email_token=AAIGPLVS3D4V6UJHOS54G5LQCGP7JA5CNFSM4IFPACYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3HPELQ#issuecomment-516878894, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIGPLTXAGNFYMF4N4X3ZS3QCGP7JANCNFSM4IFPACYA .

[dorianps]

Hmmm, I thought future dev might change this behavior too. Will take care of it once this happens. Please add this info to the commits/pulls that will carry the change.

Thanks, closing the issue.