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

Test elev.R #17

Merged
merged 66 commits into from Jul 17, 2023
Merged

Test elev.R #17

merged 66 commits into from Jul 17, 2023

Conversation

ms609
Copy link
Collaborator

@ms609 ms609 commented Jul 11, 2023

Add tests for elev(e_source = "geodata").

Modifications to elev.R:

  • Simplify re-classing of location using as(, "sf")
  • Clarify user output when bounding box invalid (and make code intent clearer)
  • Use native code to work around issues that were arising within elevation_3s()

Next, I'll try using e_source = "mapzen", which I can't get to work at the moment. It looks like elevatr::get_elev_raster() relies on rgdal, which is being deprecated as part of the sf update.

[edit] - I see that you are already aware of this issue... I'll incorporate a dependency on the beta version of elevatr.

@jamestsakalos
Copy link
Owner

I am not sure why we are receiving an error when unzipping srtm files (see the testthat results for elev).

@ms609 ms609 changed the title Test elev.R: GeoData Test elev.R Jul 17, 2023
@ms609 ms609 marked this pull request as ready for review July 17, 2023 14:16
@ms609 ms609 closed this Jul 17, 2023
@ms609 ms609 deleted the test-elev branch July 17, 2023 14:31
@jamestsakalos jamestsakalos restored the test-elev branch July 17, 2023 14:53
@jamestsakalos jamestsakalos deleted the test-elev branch July 17, 2023 14:54
@jamestsakalos jamestsakalos restored the test-elev branch July 17, 2023 14:56
@jamestsakalos
Copy link
Owner

Closed with unmerged commits.

@jamestsakalos jamestsakalos reopened this Jul 17, 2023
@jamestsakalos jamestsakalos merged commit f6612fa into master Jul 17, 2023
7 checks passed
@ms609
Copy link
Collaborator Author

ms609 commented Jul 17, 2023

Sorry – must have misread a notification – it looked like GitHub had told me it was merged. Glad you could restore!

@jamestsakalos
Copy link
Owner

No dramas! I'm totally digging how these processes work. Thanks a bunch for showing me the ropes! Honestly, I hadn't even used these features in my previous package, and now I've got a bunch of changes in mind based on what I've learned here.

@jamestsakalos jamestsakalos deleted the test-elev branch July 17, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants