Implement segmentation as part of the package

[fnattino]

No description provided.

[fnattino]

Hey @cforgaci , I finally managed to move forward with the segmentation module. Happy to hear your thoughts, for now only tests are missing.

Thinks to think on (maybe later on):

• which functions should be exported and which should remain private (we should fix it consistently everyewhere)?
• should we create a module for all the general-purpose geometry-related functions e.g. to buffer, split, find longes geometry, etc. (geometry.R or maybe even better sf.R?)

[fnattino]

TODO:

• Rename inner functions to get_corridor and get_segments
• Update vignette

[cforgaci]

@fnattino many thanks for this! I left some comments and suggestions, nothing fundamental. Can you please have a look?

[cforgaci]

Yes, this still needs to be added, but it falls outside the scope of this PR. Where should we save that? It is neither bucharest_osm, nor bucharest_dem, so it would make sense that it is a separate object called bucharest_delineation or bucharest_del which contains the corridor, segments and riverspace. Then we can use bucharest_del$corridor here. What do you think @fnattino?

[fnattino]

Maybe bucharest_delineation? Will use this here and raise an issue pointing to this thread.

[cforgaci]

• which functions should be exported and which should remain private (we should fix it consistently everyewhere)?

Maybe we expose only the top-level functions; in the segments.r module, for instance, we would export only get_segments(). We can still generate documentation for functions like clip_and_filter() and add #' @noRd to smaller helper funcions as you did. Non-exported functions would still be available to the user via the ::: namespace operator. Would that work?

• should we create a module for all the general-purpose geometry-related functions e.g. to buffer, split, find longes geometry, etc. (geometry.R or maybe even better sf.R?)

sf.R would make most sense. And I see indeed that the find_*() functions from this module could move there.

[fnattino]

Hi @cforgaci - I think I have addressed all the comments, do you want to have a look?

[cforgaci]

Looks good @fnattino! This looks ready to be merged.