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

improvements to dplyr-Spatial.r #10

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
2 participants
@jlegewie
Copy link

commented Apr 27, 2016

Hey,

I followed the discussion on the initial gist and always wanted to contribute my own take. Here are a couple of improvements (maybe not) for dplyr-Spatial.r but you should check each of them.

  1. Improved performance for select_.Spatial and filter_.Spatial
  2. Removed transmute. I don't think it's necessary to create a separate method because it just relies on mutate and select
  3. Your version of filter_.Spatial added a variable. Not sure why. This one doesn't.
  4. What does summarise_.Spatial do considering that group_by doesn't work?
  5. Added left_join. Spatial. The other joins should be easy as well. In my code, I also do left_join.SpatialLinesDataFrame <- left_join.SpatialDataFrame. Not sure how you handle that do maybe you have to change something else in a different file.

ERROR: Just saw an error in my pull-request. select_.Spatial should now take a .dots argument so that the function definition should be select_.Spatial <- function (.data, ..., .dots) { .... If you end up merging this, make sure to correct that.

@mdsumner

This comment has been minimized.

Copy link
Owner

commented Apr 28, 2016

Thanks for this!

I can't get to it right now but I will soon.

Re 4) it's a good question, to me it was "the limit" of how far you can go with Spatial objects as they are, and a reasonable illustration of simply merging all objects into one - it just collapses to a single-row Spatial*DataFrame.

@jlegewie

This comment has been minimized.

Copy link
Author

commented Apr 28, 2016

I thought about group_by + summarize for a while and I think the only possibility would be that it creates unions for the spatial polygons and summarizes the data as usual. But that get's much more involved.

@mdsumner

This comment has been minimized.

Copy link
Owner

commented Apr 29, 2016

I can't merge this as-is, we cannot use the unexported dplyr:::select_impl - thanks but I'll have to take a closer look when I can get back to it.

@jlegewie

This comment has been minimized.

Copy link
Author

commented Apr 29, 2016

Of course, I mainly wanted to start a discussion and I am not always sure myself about the best approach. Why can't you use dplyr:::select_impl? The tripple colon explicitly access unexported functions, no?

@mdsumner

This comment has been minimized.

Copy link
Owner

commented Apr 29, 2016

Ah I see, I am fumbling around a bit, I appreciate the input. You cannot
use unexported functions and pass check. I will have to spend more time
understanding nse in dplyr.
BTW I added group_by to master for Spatial, appreciate any testing.

Cheers, Mike

On Sat, 30 Apr 2016, 03:45 Joscha Legewie notifications@github.com wrote:

Of course, I mainly wanted to start a discussion and I am not always sure
myself about the best approach. Why can't you use dplyr:::select_impl?
The tripple colon explicitly access unexported functions, no?


You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#10 (comment)

Dr. Michael Sumner
Software and Database Engineer
Australian Antarctic Division
203 Channel Highway
Kingston Tasmania 7050 Australia

@mdsumner

This comment has been minimized.

Copy link
Owner

commented May 2, 2016

I've incorporated your fixes into the master branch, some things couldn't be done, but some of that may be me misunderstanding. btw, group_by is now implemented and always unions grouped geometries together by 1) decomposing to sptable 2) badging grouped branches with the same id 3) recomposing as Spatial.

@mdsumner mdsumner closed this May 2, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.