fix compatibility + fix #95#96
Conversation
|
FYI, after pulling your PR and running R CMD check on my laptop I noticed that the vignette would also need to be updated, turning:
Once this is done, the vignette runs fine, but I did notice that numbers returned are different from those returned in the original vignette (judging from the website). For example, the histogram displaying the I did not look into whether this is just because underlying data have been updated or whether something else is going on. Otherwise the checks run fine! |
|
Thanks so much for checking this PR and catching those mistakes @courtiol - I really appreciate it! |
|
After some digging, it seems the reason this passed locally was because I had cached an older version of the WDPA. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #96 +/- ##
==========================================
- Coverage 97.69% 96.80% -0.90%
==========================================
Files 10 10
Lines 608 626 +18
==========================================
+ Hits 594 606 +12
- Misses 14 20 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
You are very welcome. |
| x$MARINE[x$MARINE == "0"] <- "terrestrial" | ||
| x$MARINE[x$MARINE == "1"] <- "partial" | ||
| x$MARINE[x$MARINE == "2"] <- "marine" | ||
| if (assertthat::has_name(x, "MARINE")) { |
There was a problem hiding this comment.
I guess this all block is now useless (since MARINE no longer exists and REALM is coded with levels), unless you are trying to have your code work on different version of WDPA, but I think you are not.
|
Another issue is that not fixing geoms exposed this bug: r-spatial/lwgeom#100 Update now fixed in lwgeom devel (9 min after posting the issue!), but I noticed in the meantime that the empty polygon do not come from WDPA but are generated by |
Thanks for taking another look at this! Yeah, my goal was to update wdpar so that it would be compatible with the new version of the WDPA, as well as maintain backwards compatibility (where possible) with older versions of the WDPA. As such, I've updated the code for formatting the https://github.com/prioritizr/wdpar/blob/fix-compat/R/wdpa_clean.R#L359-L361 https://github.com/prioritizr/wdpar/blob/fix-compat/R/wdpa_clean.R#L502-L507 How does that sound? |
Thanks for catching this bug as well! Yeah, although I could try adding some extra functionality to identify records like this that are associated with geometries that are most likely incorrect, I agree that it would be better for Protected Planet to resolve this if possible. The |
|
Thanks for trying to make it backward compatible. I just wish WDPA had not change column names, but that is not on you. The decision to have wdpar returning the WDPA names of the data it reads does make sense. I did not try the new fork on old data, but I guess you did. I think that polygon issues will continue to be progressively better handled by sf, lwgeom, etc... which packages like yours use. So, it seems that it could be ready to merge 🚀 |
|
Ok great - thanks again for all your help with this! |

wdpa_read()andwdpa_clean()to be compatible with updates tothe Protected Planet database. Thanks to Joe Gosling for providing detailed
information on the database changes.
wdpa_clean()so that it shows clearer progress messages for erasingspatial overlaps, throws a warning if attempting to erase overlaps from
particularly large datasets, trims white space characters from all
fields containing character values, and can optionally skip steps for
repairing geometries if specified by the user (per the new
repair_geometriesparameter) (feature request: make geometry repair optional inwdpa_clean()#95). Thanks to Alexandre Courtiol(@courtiol) for feature suggestion.