MikkoVihtakari / ggOceanMaps

Plot oceanographic research data on maps using ggplot2
GNU General Public License v3.0
45 stars 7 forks source link

Issue CRS (auto_limits function) #7

Closed yreecht closed 3 years ago

yreecht commented 3 years ago

Hi Mikko,

Thanks for sharing this useful package.

The default value proj.in = "EPSG:4326" in the function auto_limits does not work on all systems. This is because of a non-systematic support of SRS_string in the package sp (projargs is passed to SRS_string if in this format and sp >= 1.4-4):

SRS_string: default NULL, only used when rgdal is built with PROJ >= 6 and GDAL >= 3; a valid WKT string or SRS definition such as "EPSG:4326" or "ESRI:102761"

This is in particular an issue when building packages under some LTS Linux distributions, where it makes the function basemap unusable.

An easy workaround would be, I think, to use proj.in = "+init=epsg:4326" instead.

Best wishes, Yves

yreecht commented 3 years ago

Okay, I realise that this is probably linked to the shifting to the PROJ6/GDAL3 system you mention on the front page. Sorry if the issue is irrelevant.

Is this kind of projection specification ("+init=epsg:XXXX") supposed to eventually become obsolete?

Temporarily, I've made a fork where I have replaced all occurrences of EPSG: by the above +init=epsg:, so I can keep using it. Seems to work fine so far.

MikkoVihtakari commented 3 years ago

Thanks for reporting the issue. I think sf, sp, rgdal, and rgeos developers are rolling to PROJ >= 6 and GDAL >= 3, and the support for older versions will be ditched eventually. Is there any way you could update PROJ and GDAL on your system?

As a work-around, one could try to detect which version of PROJ and GDAL the system uses and adjust the argument accordingly. Please tell me, if you'll find a test that works. I think I have a few drafts somewhere. Need to find them. I'll do a serious attempt to make the package CRAN compatible again later this autumn. While at it, I'll try to fix this issue. Right now I am swamped with other tasks.

yreecht commented 3 years ago

Hi Mikko,

Thanks for looking into it.

An idea of function that return the adequate string depending on the installed libraries:


CRSargs <- function(epsg = 4326)
{
    ## Minimal versions for CRS() to support SRS_string:
    minVersions <- c("GDAL" = "3.0", "PROJ" = "6.0", "sp" = "1.4-4")

    res <- sapply(names(minVersions),
                  function(i)
           {
               utils::compareVersion(rgdal::rgdal_extSoftVersion()[i],
                                     minVersions[i])
           })

    return(paste0(ifelse(any(res < 0), "+init=epsg:", "EPSG:"),
                  epsg))
}
MikkoVihtakari commented 3 years ago

Thanks! The only issue here is that it requires importing rgdal. I have tried to remove that dependency and replace it with sf. The sf package might be unstable on older Linux etc distributions. Not entirely sure what is the best way forward. Anyway, sf must have function to test the GDAL version too (if we'll go that way instead of continuing with Roger's packages).

More about sf here: https://github.com/MikkoVihtakari/ggOceanMaps/issues/5

yreecht commented 3 years ago

Yes, sorry, I missed that part!

Then:

CRSargs <- function(epsg = 4326)
{
    ## Minimal versions for CRS() to support SRS_string:
    minVersions <- c("GDAL" = "3.0", "PROJ" = "6.0", "sp" = "1.4-4")

    installed <- c(sf::sf_extSoftVersion(),
                   sp = as.character(packageVersion("sp")))[names(minVersions)]

    res <- sapply(names(minVersions),
                  function(i)
           {
               utils::compareVersion(installed[i],
                                     minVersions[i])
           })

    return(paste0(ifelse(any(res < 0), "+init=epsg:", "EPSG:"),
                  epsg))
}
MikkoVihtakari commented 3 years ago

Thank you. I'll report back once this is incorporated.

yreecht commented 3 years ago

I have implemented it in my fork... I can complete the doc and send a pull request next week if you wish.

Do you have some test scripts I can try to validate that all runs smoothly?

MikkoVihtakari commented 3 years ago

Yes. I have automatic tests for the most common OS. Here: https://github.com/MikkoVihtakari/ggOceanMaps/blob/master/.github/workflows/R-CMD-check.yaml

MikkoVihtakari commented 3 years ago

@yreecht, I pushed a simplified solution to the main branch. I don't have a machine to test it. Would you mind?

MikkoVihtakari commented 3 years ago

Seems like the simplified solution may not work, but perhaps your solution will. Worth trying at least. When you'll have time, could you kindly create a pull request to this branch: https://github.com/MikkoVihtakari/ggOceanMaps/tree/experimental ?

Let's make sure that it works before we merge with the main branch.

yreecht commented 3 years ago

Done. Sorry for the numerous lines with only end of line space removal... happened automatically in my editor.

MikkoVihtakari commented 3 years ago

It works! I'll push the changes to the main branch and resubmit to CRAN. May I add you as an author for the package @yreecht?

ggOceanMaps 1.1.15: OK

Build ID:ggOceanMaps_1.1.15.tar.gz-b1136e2c6e5d45d2bae6b271e3f52502Platform:Oracle Solaris 10, x86, 32 bit, R-releaseSubmitted:25 minutes 51.6 seconds agoBuild time:25 minutes 47.6 seconds | Build ID:ggOceanMaps_1.1.15.tar.gz-b1136e2c6e5d45d2bae6b271e3f52502Platform:Oracle Solaris 10, x86, 32 bit, R-releaseSubmitted:25 minutes 51.6 seconds agoBuild time:25 minutes 47.6 seconds | Build ID: | ggOceanMaps_1.1.15.tar.gz-b1136e2c6e5d45d2bae6b271e3f52502 | Platform: | Oracle Solaris 10, x86, 32 bit, R-release | Submitted: | 25 minutes 51.6 seconds ago | Build time: | 25 minutes 47.6 seconds Build ID:ggOceanMaps_1.1.15.tar.gz-b1136e2c6e5d45d2bae6b271e3f52502Platform:Oracle Solaris 10, x86, 32 bit, R-releaseSubmitted:25 minutes 51.6 seconds agoBuild time:25 minutes 47.6 seconds | Build ID: | ggOceanMaps_1.1.15.tar.gz-b1136e2c6e5d45d2bae6b271e3f52502 | Platform: | Oracle Solaris 10, x86, 32 bit, R-release | Submitted: | 25 minutes 51.6 seconds ago | Build time: | 25 minutes 47.6 seconds Build ID: | ggOceanMaps_1.1.15.tar.gz-b1136e2c6e5d45d2bae6b271e3f52502 Platform: | Oracle Solaris 10, x86, 32 bit, R-release Submitted: | 25 minutes 51.6 seconds ago Build time: | 25 minutes 47.6 seconds

yreecht commented 3 years ago

Great news :)

May I add you as an author for the package @yreecht?

Sure, if you think it is substantial enough a contribution. Thank you.

MikkoVihtakari commented 3 years ago

By adding you, I am hoping that you'll contribute in the future too ;) Kind of fed up maintaining this package alone. It takes a too long time seeing that there is no official funding (project) for it.

MikkoVihtakari commented 3 years ago

This issue should be addressed here: https://github.com/MikkoVihtakari/ggOceanMaps/commit/24780292d6750d9c530f8d44dad56fefb29cc878

I'll close the issue. Please reopen if the issue prevails.