geojsonio
geojsonio copied to clipboard
sf CRS fixes
Address #159:
- Don't access crs directly with
attrs()
, rather always usest_crs()
- Don't access crs slots with
[[
, rather use the$
methods.
Codecov Report
Merging #169 into master will increase coverage by
3.33%
. The diff coverage is100.00%
.
@@ 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.
thanks @ateucher ! Does this work with the old sf version too?
It should (by old you mean 0.8-1?)
These github actions on mac are going to be the death of me
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
@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
Given that this change has some rough edges, i'm going to submit with these tests commented out
I think that's a good idea, sorry I don't have a ton of time today to spend on it, but can try?
no worries, already submitted