R devel - May 2004 - points(*, pch=NA) does *not* not draw the point (PR#6876)

We say in ?points  that 'pch' (among others) can be set to NA
for omitting a point.

While this works in cases where there's at least one point left
to draw, it fails in a simple case like this :
> plot(1, pch = NA)
Error in plot.xy(xy.coords(x, y), type = type, pch = pch, col = col, bg = bg,  :
	invalid plotting symbol

Both in R-patched or R-devel.
A simple workaround {hinting at how to fix the C code} is
> plot(1, pch = as.integer(NA))
So this is easily fixable (and I'll commit a patch soon).

----

A bit more problematic *) but not a bug in a very strict sense
is the following : 

 > plot(1, pch = as.character(NA))
and
 > plot(1:4, pch=c("o",".", NA, "x"))

which are identical to

 > plot(1, pch = "N")
and
 > plot(1:4, pch=c("o",".", "N",
"x"))

but should either produce a warning (additional to using "N") or
rather also treat the NA as "missing", i.e., not drawing a point.


Martin Maechler


---------------
*) because this isn't spotted when used in a context of
   plotting many things

On Wed, 12 May 2004 maechler@stat.math.ethz.ch wrote:
> We say in ?points  that 'pch' (among others) can be set to NA
> for omitting a point.
I don't think we actually do.  We say 

     Points whose 'x', 'y', 'pch', 'col' or
'cex' value is 'NA' are
     omitted from the plot.

and earlier

     either be a 'character' or an integer code

I read that to mean that as.logical(NA) is incorrect, but that 
as.character(NA) is correct and should result in the point being omitted.

In short, I disagree as to which is a very strict bug (although it makes 
sense to allow logical NAs, of course).

The problems are both in FixupPch in src/main/plot.c.  Can I leave you to 
fix both?
> While this works in cases where there's at least one point left
> to draw, it fails in a simple case like this :
> 
> > plot(1, pch = NA)
> Error in plot.xy(xy.coords(x, y), type = type, pch = pch, col = col, bg =
bg,  :
> 	invalid plotting symbol
> 
> Both in R-patched or R-devel.
> A simple workaround {hinting at how to fix the C code} is
> 
> > plot(1, pch = as.integer(NA))
> 
> So this is easily fixable (and I'll commit a patch soon).
> 
> ----
> 
> A bit more problematic *) but not a bug in a very strict sense
> is the following : 
> 
>  > plot(1, pch = as.character(NA))
> and
>  > plot(1:4, pch=c("o",".", NA, "x"))
> 
> which are identical to
> 
>  > plot(1, pch = "N")
> and
>  > plot(1:4, pch=c("o",".", "N",
"x"))
> 
> but should either produce a warning (additional to using "N") or
> rather also treat the NA as "missing", i.e., not drawing a point.
-- 
Brian D. Ripley,                  ripley@stats.ox.ac.uk
Professor of Applied Statistics,  http://www.stats.ox.ac.uk/~ripley/
University of Oxford,             Tel:  +44 1865 272861 (self)
1 South Parks Road,                     +44 1865 272866 (PA)
Oxford OX1 3TG, UK                Fax:  +44 1865 272595

>>>>> "BDR" == Prof Brian Ripley
<ripley@stats.ox.ac.uk>
>>>>>     on Wed, 12 May 2004 15:20:56 +0100 (BST) writes:
    BDR> On Wed, 12 May 2004 maechler@stat.math.ethz.ch wrote:
    >> We say in ?points that 'pch' (among others) can be set to
    >> NA for omitting a point.

    BDR> I don't think we actually do.  We say

    BDR>      Points whose 'x', 'y', 'pch',
'col' or 'cex' value
    BDR> is 'NA' are omitted from the plot.

    BDR> and earlier

    BDR>      either be a 'character' or an integer code

    BDR> I read that to mean that as.logical(NA) is incorrect,
    BDR> but that as.character(NA) is correct and should result
    BDR> in the point being omitted.

    BDR> In short, I disagree as to which is a very strict bug
    BDR> (although it makes sense to allow logical NAs, of
    BDR> course).

You're right, with both statements.

    BDR> The problems are both in FixupPch in src/main/plot.c.
    BDR> Can I leave you to fix both?

yes, I've been in there anyway.

There's one remaining design decision:

At the moment I'd go to allow 'logical' and coerce that to
integer (as in many other S code places).  
Alternatively, more strict behavior would only allow NA logicals
and give an error for TRUE or FALSE entries in pch vectors.
I currently think it's not worth the extra check.

    >> While this works in cases where there's at least one
    >> point left to draw, it fails in a simple case like this :
    >> 
    >> > plot(1, pch = NA) Error in plot.xy(xy.coords(x, y),
    >> type = type, pch = pch, col = col, bg = bg, : invalid
    >> plotting symbol
    >> 
    >> Both in R-patched or R-devel.  A simple workaround
    >> {hinting at how to fix the C code} is
    >> 
    >> > plot(1, pch = as.integer(NA))
    >> 
    >> So this is easily fixable (and I'll commit a patch soon).
    >> 
    >> ----
    >> 
    >> A bit more problematic *) but not a bug in a very strict
    >> sense is the following :
    >> 
    >> > plot(1, pch = as.character(NA)) and > plot(1:4,
    >> pch=c("o",".", NA, "x"))
    >> 
    >> which are identical to
    >> 
    >> > plot(1, pch = "N") and > plot(1:4,
pch=c("o",".", "N",
    >> "x"))
    >> 
    >> but should either produce a warning (additional to using
    >> "N") or rather also treat the NA as "missing",
i.e., not
    >> drawing a point.

On Wed, 12 May 2004, Martin Maechler wrote:
> >>>>> "BDR" == Prof Brian Ripley
<ripley@stats.ox.ac.uk>
> >>>>>     on Wed, 12 May 2004 15:20:56 +0100 (BST) writes:
> 
>     BDR> On Wed, 12 May 2004 maechler@stat.math.ethz.ch wrote:
>     >> We say in ?points that 'pch' (among others) can be set
to
>     >> NA for omitting a point.
> 
>     BDR> I don't think we actually do.  We say
> 
>     BDR>      Points whose 'x', 'y', 'pch',
'col' or 'cex' value
>     BDR> is 'NA' are omitted from the plot.
> 
>     BDR> and earlier
> 
>     BDR>      either be a 'character' or an integer code
> 
>     BDR> I read that to mean that as.logical(NA) is incorrect,
>     BDR> but that as.character(NA) is correct and should result
>     BDR> in the point being omitted.
> 
>     BDR> In short, I disagree as to which is a very strict bug
>     BDR> (although it makes sense to allow logical NAs, of
>     BDR> course).
> 
> You're right, with both statements.
> 
>     BDR> The problems are both in FixupPch in src/main/plot.c.
>     BDR> Can I leave you to fix both?
> 
> yes, I've been in there anyway.
> 
> There's one remaining design decision:
> 
> At the moment I'd go to allow 'logical' and coerce that to
> integer (as in many other S code places).  
> Alternatively, more strict behavior would only allow NA logicals
> and give an error for TRUE or FALSE entries in pch vectors.
> I currently think it's not worth the extra check.
Here it probably makes more sense to coerce to character, and given the 
ambiguity I would allow only an all-NA logical vector.

Brian

-- 
Brian D. Ripley,                  ripley@stats.ox.ac.uk
Professor of Applied Statistics,  http://www.stats.ox.ac.uk/~ripley/
University of Oxford,             Tel:  +44 1865 272861 (self)
1 South Parks Road,                     +44 1865 272866 (PA)
Oxford OX1 3TG, UK                Fax:  +44 1865 272595

>>>>> "BDR" == Brian D Ripley
<ripley@stats.ox.ac.uk>
>>>>>     on Wed, 12 May 2004 16:45:49 +0200 (CEST) writes:
    BDR> On Wed, 12 May 2004, Martin Maechler wrote:

   ...... 

    >> There's one remaining design decision:
    >> 
    >> At the moment I'd go to allow 'logical' and coerce that
    >> to integer (as in many other S code places).
    >> Alternatively, more strict behavior would only allow NA
    >> logicals and give an error for TRUE or FALSE entries in
    >> pch vectors.  I currently think it's not worth the extra
    >> check.

    BDR> Here it probably makes more sense to coerce to
    BDR> character, and given the ambiguity I would allow only
    BDR> an all-NA logical vector.

Ok, I've just committed this (and tests for it) to R-patched and
R-devel.

Bug closed.

Martin