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

SOmap_auto to handle target=NULL #50

Open
raymondben opened this issue Jun 4, 2019 · 13 comments
Open

SOmap_auto to handle target=NULL #50

raymondben opened this issue Jun 4, 2019 · 13 comments
Assignees

Comments

@raymondben
Copy link
Member

Reminder for the thing we discussed the other day: SOmap_auto(raster_layer, target = NULL) should inherit target from raster_layer, not try and guess its own.

@mdsumner
Copy link
Member

mdsumner commented Jun 4, 2019

that's done, should be

  library(SOmap)
  x <- raster::raster(system.file("nc/reduced.nc", package = "stars"), varname = "sst")
  a <- SOmap_auto(x, target = NULL)
  a

but I'm getting fig errors on plot ...

@mdsumner
Copy link
Member

mdsumner commented Jun 4, 2019

Wowsers, if I don't "rotate()" I get freaky fig errors ...

library(SOmap)
#> Loading required package: raster
#> Loading required package: sp
x <- raster::rotate(raster::raster(system.file("nc/reduced.nc", package = "stars"), varname = "sst"))
#> Loading required namespace: ncdf4
a <- SOmap_auto(x, target = NULL)
#> Warning in CPL_proj_direct(as.character(c(from[1], to[1])),
#> as.matrix(pts)): one or more projected point(s) not finite
#> dist is assumed to be in decimal degrees (arc_degrees).
#> although coordinates are longitude/latitude, st_intersection assumes that they are planar
a

Created on 2019-06-04 by the reprex package (v0.3.0)

@Maschette
Copy link

Was I part of this discussion, I forget. Sounds fun!

Is it getting confused by being over the northern hemisphere?

@Maschette Maschette reopened this Jun 4, 2019
@mdsumner
Copy link
Member

mdsumner commented Jun 4, 2019

More examples, found a bug

library(SOmap)
#> Loading required package: raster
#> Loading required package: sp
a <- SOmap_auto(ice, target = NULL)
a

Created on 2019-06-04 by the reprex package (v0.3.0)

@Maschette
Copy link

What is the reason for setting target = NULL? surely that tells it there is no target?

@mdsumner
Copy link
Member

mdsumner commented Jun 4, 2019

It tells it to use the input raster grid as-is even if that grid is in longlat.

@Maschette
Copy link

Ok so it is not actually target=NULL but target=projection(x) internally.
That makes sense but I think the default should still be our standard projection.

@raymondben
Copy link
Member Author

If x as a raster object and target is left as its default (stere) then SOmap_auto(x) will use the extent of x but NOT its projection - i.e. following your "default should still be our standard projection". If you set target = NULL it is saying to use the projection of x (and that is equivalent to specifying target = projection(x)).

@Maschette
Copy link

Weird output for the new SOmap_auto with class of spatialpointsdataframe

library(rgdal)
#> Loading required package: sp
#> rgdal: version: 1.4-4, (SVN revision 833)
#>  Geospatial Data Abstraction Library extensions to R successfully loaded
#>  Loaded GDAL runtime: GDAL 2.2.3, released 2017/11/20
#>  Path to GDAL shared files: C:/Users/dale_mas/Documents/R/R-3.6.0/library/rgdal/gdal
#>  GDAL binary built with GEOS: TRUE 
#>  Loaded PROJ.4 runtime: Rel. 4.9.3, 15 August 2016, [PJ_VERSION: 493]
#>  Path to PROJ.4 shared files: C:/Users/dale_mas/Documents/R/R-3.6.0/library/rgdal/proj
#>  Linking to sp version: 1.3-1
library(SOmap)
#> Loading required package: raster
#> Registered S3 methods overwritten by 'ggplot2':
#>   method         from 
#>   [.quosures     rlang
#>   c.quosures     rlang
#>   print.quosures rlang
brokew<-readOGR("C:\\Users\\dale_mas\\Documents\\Projects\\Mapping\\data\\Krill_surveys_EastAntarctica\\Krill_surveys_EastAntarctica\\BROKEWest_transects.shp")
#> OGR data source with driver: ESRI Shapefile 
#> Source: "C:\Users\dale_mas\Documents\Projects\Mapping\data\Krill_surveys_EastAntarctica\Krill_surveys_EastAntarctica\BROKEWest_transects.shp", layer: "BROKEWest_transects"
#> with 3890 features
#> It has 6 fields
#> Integer64 fields read as strings:  transect segment
plot(brokew)

SOmap_auto(brokew)
#> Error in st_cast_sfc_default(x): list item(s) not of class sfg

Created on 2019-06-04 by the reprex package (v0.3.0)

@mdsumner
Copy link
Member

mdsumner commented Jun 4, 2019

I can't reproduce that, can you print a summary? (or send me the data)

library(raster)
print(brokew)

@mdsumner
Copy link
Member

mdsumner commented Jun 4, 2019

Also I'm sympathetic to the issue of SOmap being about stereographic. Maybe it's time to take the general stuff out.

The issue atm is

  • we default to projecting input data, with target = stereographic (1)
  • we determine the extent and orientation from your data (2)
  • we provide custom choice of projection type (target), optionally centre, and optionally full specification (3)
  • if the input data is projected, we use its extent and projection, ignoring target if it is the same type (4)
  • or, if not the same type we work out a new extent in the input target (5)
  • if the input data is not projected, we go to (2), unless target = NULL (6)

It's helpful to write that all down, I want to flow-diagram it.

I feel like customizing target in (5) is too much, maybe we shouldn't allow that and just set target = "stere" and rather than NULL only allow target = "source".

But, this is functional enough that we should just push through the gg stuff as well, then review.

(I just love being able to do anything I want with any input, but it is getting complicated again. If we can seal this off sensibly I'll pull out the general stuff and make "panmap" or something)

@raymondben
Copy link
Member Author

The "fig errors on plot" (second comment, above) are fixed in ceffb43. But there is a lot more discussion here, what do we want to do with it? @mdsumner @Maschette

@Maschette
Copy link

Kill it with fire.
The actual issue is fixed. It does highlight though a need for a check on the data, in what was the first example the raster is 0-360, everything in SOmap is -180 to 180 so it plots half the continents but not the other. We could add a warning telling people what the issue is.

@Maschette Maschette reopened this Feb 4, 2020
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

No branches or pull requests

3 participants