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

Map fetch mvt #378

Merged
merged 13 commits into from Oct 10, 2019

Conversation

@jeroen
Copy link
Member

commented Oct 5, 2019

No description provided.

@jeroen jeroen force-pushed the map-fetch-mvt branch from da32a4d to 59d6848 Oct 5, 2019

query <- rgbif_compact(list(srs = srs, taxonKey = taxonKey,

input_srs <- ifelse(grepl("mvt", format), "EPSG:3857", srs)

This comment has been minimized.

Copy link
@sckott

sckott Oct 7, 2019

Member

@jeroen problem is i think that here we're forcing srs to be 3857 if mvt is requested, but then below srs is used when passing to protolite::read_mvt_sf - so you always request 3857 from GBIF, but then protolite uses whatever the user passed in. Is that what you intended?

This comment has been minimized.

Copy link
@jeroen

jeroen Oct 7, 2019

Author Member

Yes, protolite only understands 3857 input data, so we always need that. And then we ask protolite::read_mvt_sf to convert it to whatever output the user requested.

This comment has been minimized.

Copy link
@sckott

sckott Oct 7, 2019

Member

okay, i think we should then only allow 3857 when format="mvt", correct?

This comment has been minimized.

Copy link
@jeroen

jeroen Oct 7, 2019

Author Member

Yes we always use "EPSG:3857" as the input data that we get from gbif. The output crs may be be anything.

This comment has been minimized.

Copy link
@sckott

sckott Oct 7, 2019

Member

ah, so the user can still set a different srs for the output, the srs that's passed to protolite::read_mvt_sf?

This comment has been minimized.

Copy link
@jeroen

jeroen Oct 7, 2019

Author Member

Indeed, in case of mvt, the input data is always 3857 but we can transform it into the desired coordinate system after parsing it.

@codecov-io

This comment has been minimized.

Copy link

commented Oct 10, 2019

Codecov Report

Merging #378 into master will increase coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #378     +/-   ##
=========================================
+ Coverage   65.47%   65.57%   +0.1%     
=========================================
  Files          52       52             
  Lines        2323     2327      +4     
=========================================
+ Hits         1521     1526      +5     
+ Misses        802      801      -1
Impacted Files Coverage Δ
R/map_fetch.R 79.51% <100%> (+2.59%) ⬆️
R/occ_download.R 55.2% <0%> (-0.36%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c4ad6e...09bd573. Read the comment docs.

@sckott sckott added this to the v1.4 milestone Oct 10, 2019
@sckott sckott added the maps-api label Oct 10, 2019
@sckott sckott merged commit daa2e45 into master Oct 10, 2019
0 of 4 checks passed
0 of 4 checks passed
continuous-integration/appveyor/branch Waiting for AppVeyor build to complete
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.