Bug 16423 - canCoerce() reports false negative then changes its mind -- selectMethod() bug
Summary: canCoerce() reports false negative then changes its mind -- selectMethod() bug
Status: ASSIGNED
Alias: None
Product: R
Classification: Unclassified
Component: S4methods (show other bugs)
Version: R-devel (trunk)
Hardware: All All
: P5 major
Assignee: R-core
URL:
Depends on:
Blocks:
 
Reported: 2015-06-12 22:04 UTC by Hervé Pagès
Modified: 2015-06-16 13:32 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hervé Pagès 2015-06-12 22:04:06 UTC
Just discovered the existence of canCoerce() and gave it a try:

  setClass("A", representation(aa="character"))
  setClass("B", contains="A")

  canCoerce(new("A"), "B")  # FALSE

However:

  as(new("A"), "B")  # works!

And now, canCoerce() gives a different answer:

  canCoerce(new("A"), "B")  # TRUE

Not too surprising given the fact that canCoerce() uses selectMethod()
behind the scene and that selectMethod() has been known for many years
now to not always tell the truth i.e. to not reflect exactly what the
real method dispatch mechanism is going to call.

So I'll stick to 'try(as(x, cl))' for now. Note that even if a coerce
method exists (or will be automatically generated), coercion can still
fail. So 'try(as(x, cl))' will almost always be better than 'canCoerce(x, cl)'.
Also if I care about strict coercion being successful, I need to do something
like:

  y <- try(as(x, cl))
  if (inherits(y, "try-error") || class(y) != "cl") {
      ... handle failure to coerce strictly
  }

canCoerce() of course has no way to tell me if I can coerce strictly.

Thanks,
H.
Comment 1 Michael Lawrence 2015-06-14 12:52:06 UTC
Perhaps we need a generic form of canCoerce, so that custom logic can be defined for particular coercions. This could fallback to try(as(from,to)) but would generally be more efficient and would be less likely to mask bugs.
Comment 2 Hervé Pagès 2015-06-15 20:19:40 UTC
Hi Michael,

canCoerce() could do !inherits(try(as(from, to), silent=TRUE), "try-error").
Or it could be made a generic function that fallbacks to that but I wonder how
many developers would bother define methods for it (they've already too many
things to take care of).

Anyway, it seems that the typical usage of canCoerce() is to check before
actually doing the coercion:

  if (canCoerce(from, to)) {
    y <- as(from, to)
  } else {
    ...
  }

Maybe the above is a little bit more readable than:

  y <- try(as(from, to), silent=TRUE)
  if (inherits(y, "try-error")) {
    ...
  }

but not much. However it has 2 downsides:

1) If coercion works, then it's performed twice.
2) If canCoerce() is a generic, I cannot really trust it to do the right thing
   anymore (i.e. there is the possibility that it returns false positives or
   false negatives).

So again, I really like the fact that try(as(from, to)) never lies. I agree
that it has the potential to mask bugs but maybe that means we need better
error handling capabilities?

H.
Comment 3 Michael Lawrence 2015-06-16 00:56:33 UTC
Developers could factor out the checks they already have in their coerce method into a canCoerce method. The default canCoerce() method would return TRUE if a coerce method exists (which might need fixing). It just seems wrong to conflate failure of a function call with whether coercion is conceptually possible.
Comment 4 Hervé Pagès 2015-06-16 02:38:03 UTC
> It just seems wrong to conflate failure of a function call with whether
> coercion is conceptually possible.

I guess that's another argument for not having canCoerce() fallback to
try(as(from,to)). I can do try(as(from,to)) myself if I want to know whether
that works or not, I don't need canCoerce() for that.

If we are back to the idea that canCoerce() should fallback on checking for
existence of a coerce method, that's fine with me. I guess what really matters
to me is that selectMethod() could be trusted.

Thanks,
H.
Comment 5 Martin Maechler 2015-06-16 13:32:03 UTC
canCoerce() was introduced by me in 2006 ... as simple utility function with relatively straightforward definition.... ((but not absolutely trivial, so "worth" to be made available as function)).

Of course, at the time I was not aware of the fact  "that selectMethod() has been known for many years now to not always tell the truth".

I really think canCoerce() does the right thing and if it is buggy, it's a consequence of selectMethod() being buggy.

Note that John and I did fix one or actually two cases of selectMethod() "failure" in Feb.2012, 
r58493 | maechler | 2012-02-26 16:08:45 +0100 
r58511 | maechler | 2012-02-27 22:56:13 +0100
... also because we did agree that selectMethod() should work ... and actually at the time I had hoped selectMethod() *was* finally working...

In this sense, this is a bug report about selectMethod() "only"
and if you search 'selectMethod' in the bug archive, you find PR# 16194
(https://bugs.r-project.org/bugzilla/show_bug.cgi?id=16194), also by you.

Let's start a concerted effort on getting  selectMethod() to "tell the truth",
and maybe use a different medium than the bugzilla web interface for that.