Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update docs for od_aggregate and new function #165

Merged
merged 12 commits into from
Dec 13, 2016

Conversation

Robinlovelace
Copy link
Member

Have wanted to create a quadrant() function for a while. Here's a simple one. Plus simple additions to docs of od_aggregate.

@Robinlovelace
Copy link
Member Author

Note @nikolai-b this builds in 4 min instead of 17. We're caching packages in travis now, as you said:

cache = true

@richardellison
Copy link
Collaborator

Looks good, except what happens if the latitude or longitude for a zone centroid is exactly the same as the aggregate centroid? Admittedly it is a bit of an edge case but it is possible.

@Robinlovelace
Copy link
Member Author

It returns NAs, as now documented here: 8436fde

That results in this:

rplot

@richardellison
Copy link
Collaborator

OK. I'll merge now. Might be useful to add an option later to name those cases as 'N','E','S','W' or alternatively include them with one of the quadrants. I might add that in a future update, that could also include allowing any number of sections (not just quadrants) as I imagine it might be quite useful.

@richardellison richardellison merged commit e09fad4 into ropensci:master Dec 13, 2016
@Robinlovelace
Copy link
Member Author

I think any things that can help break up a city into understandable chunks will be useful.

In fact, the process of creating that example with rasters makes me think that could be a good way forward - note we could drastically reduce the number of lines of code of that function, and make it more generalisable, by using a raster with ncol columns and nrow rows to do the subsetting.

What I've created is just the simplest case where both = 2.

@Robinlovelace
Copy link
Member Author

So yes, very happy for you to refactor this - mine was just a 1st try for the purposes of another example.

@richardellison
Copy link
Collaborator

I hadn't thought of that. My initial thinking was simply dividing regions into pie-shaped slices around the centroid but now that you mention it I think having an option for ncol and nrow would also be useful. Might be better off as two different functions? One with nrow and ncol and another that divides it based on direction (similar to the existing function but gives the options of additional breaks (N, NE, E, SE, S, etc.) or to use a possibly extreme example 0-10 degrees, 10-20, 20-30, etc.

@Robinlovelace
Copy link
Member Author

Robinlovelace commented Dec 13, 2016

I had also thought of the 10, 20, 30, 45 and 90 (what we have currently) degree options would all be useful. N degrees or rather n slices would be the generic version.

I think separate functions that do that make sense: I imagine functions with intuitive names like city_slice, and city_block could be called depending on parameters by a general function such as city_segment. But naming is subjective and should come secondary to the actual code.

Happy if you work on the slices - I could work on the raster cell-based implementation.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants