avoiding low-level grid unit access

[pmur002]

Hi

In geom_arrow.R (and aslight variation in geom_streamline.R) , you have ...

  arrow$length <- unit(as.numeric(arrow$length)*mag, attr(arrow$length, "unit"))

... which will fail if we change the internal implementation of 'grid' units (which is something we are currently trying out). I believe your code could (and should) be replaced by ...

  arrow$length <- mag*arrow$length

Paul

[eliocamp]

Hi, Paul! Thanks for the heads up. I've tried the change and realised that there was a reason I went with that circuitous route. This is how it works now:

library(data.table)
library(ggplot2)

data(seals)

ggplot(seals, aes(long, lat)) +
    geom_vector(aes(dx = delta_long, dy = delta_lat), skip = 1) +
    scale_mag()

Note that arrow lengths are proportional to magnitude.

And here's how it works out using the simpler code:

Note that they are all the same length.

[pmur002]

Thanks for checking this out.

The below is what I get from your code above ... ... and that is with the attached package metR_0.4.0.tar.gz, which has this code on line 217 of geom_arrow.R ...

     arrow$length <- mag*arrow$length

Is that the same as the change that you tried ?

[eliocamp]

Yes, it's exactly what I've changed. That's very weird that you get a different result :thinking:

I've created a branch with the change. Do you get the same result? Skipping

library(data.table)
library(ggplot2)
library(metR) # devtools::install_github("eliocamp/metR@grid-arrow")

data(seals)

ggplot(seals, aes(long, lat)) +
    geom_vector(aes(dx = delta_long, dy = delta_lat), skip = 1) +
    scale_mag()

^{Created on 2019-10-07 by the reprex package (v0.3.0)}

[pmur002]

Thanks for sticking with this. I have found the (glaring) difference in what I am doing. If I run your example with the 'metR' branch on the current r-devel R version I get the same as you ... It is only when I run the 'metR' branch on the R version I have with the new 'grid' unit implementation that I get the right result ...

So this is hopefully a useful modification to keep in hand for when (if) we shift over to the new units in 'grid' in an R release, but not a good idea to do yet.

Thanks again for engaging with this suggestion.

[eliocamp]

Thanks! Then I should add a check for the grid version when the new implementation come out. Do you know which version number will feature the change?

[pmur002]

Sorry, it is not certain yet. I will be in touch once we know which R version this will be in.

[eliocamp]

Awesome. As soon as it does, I'll add the fix. Thanks again!