geojsonio icon indicating copy to clipboard operation
geojsonio copied to clipboard

sf CRS fixes

Open ateucher opened this issue 4 years ago • 9 comments

Address #159:

  • Don't access crs directly with attrs(), rather always use st_crs()
  • Don't access crs slots with [[, rather use the $ methods.

ateucher avatar Apr 03 '20 00:04 ateucher

Codecov Report

Merging #169 into master will increase coverage by 3.33%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #169      +/-   ##
==========================================
+ Coverage   51.44%   54.77%   +3.33%     
==========================================
  Files          30       30              
  Lines        1318     1318              
==========================================
+ Hits          678      722      +44     
+ Misses        640      596      -44     
Impacted Files Coverage Δ
R/crs_convert.R 95.23% <100.00%> (+95.23%) :arrow_up:
R/zzz.r 58.19% <0.00%> (+0.40%) :arrow_up:
R/geojson_list.R 65.48% <0.00%> (+2.65%) :arrow_up:

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 fd61fdc...f5e8ae3. Read the comment docs.

codecov-io avatar Apr 03 '20 00:04 codecov-io

thanks @ateucher ! Does this work with the old sf version too?

sckott avatar Apr 03 '20 00:04 sckott

It should (by old you mean 0.8-1?)

ateucher avatar Apr 03 '20 04:04 ateucher

These github actions on mac are going to be the death of me

ateucher avatar Apr 03 '20 04:04 ateucher

hmm, this one's failing https://github.com/ropensci/geojsonio/pull/169/checks?check_run_id=556636132 but i don't see a way to tell what version of sf it has

sckott avatar Apr 03 '20 14:04 sckott

@ateucher getting lots of warnings on the crs_convert tests like

the following proj4string elements are ignored: +proj=longlat +datum=WGS84 +no_defs +ellps=WGS84 +towgs84=0,0,0 ; remove the +init=epsg:XXXX to undo this

sckott avatar Apr 03 '20 14:04 sckott

Given that this change has some rough edges, i'm going to submit with these tests commented out

sckott avatar Apr 03 '20 14:04 sckott

I think that's a good idea, sorry I don't have a ton of time today to spend on it, but can try?

ateucher avatar Apr 03 '20 15:04 ateucher

no worries, already submitted

sckott avatar Apr 03 '20 15:04 sckott