maxcovr icon indicating copy to clipboard operation
maxcovr copied to clipboard

Appropriately handle `lat` and `long` columns and add flexibility

Open njtierney opened this issue 7 years ago • 3 comments

At the moment maxcovr assumes that all columns with latitude and longitude information are named lat and long.

This is not ideal, as it throws a cryptic error message when these are not given the appropriate names.

There should be three steps in refactoring:

  1. Add a stop function that stops the analysis if things are not named lat and long. This can then be used in each function that uses that info.

  2. Allow the user to specify their own lat and long arguments

  3. Cleverly detect different variations on lat and long as leaflet does and display a nice message

njtierney avatar Jul 19 '18 05:07 njtierney

I've got this kind of thing in dodgr with an internal dodgr_graph_cols function, which relies on a whole bunch of helper functions here. It's kinda messy, but works really well at auto-identifying standard columns based on unknown and variable input names. The idea is to return a list so everything else then works like this:

cols <- dodgr_graph_cols (graph)
lon <- my_input [[cols$lon]]

... or whatever. Point is, it's robust and flexible: Write a function to auto-identify the columns you need; return those as list items; then use code like the above to extract when needed.

mpadge avatar Jun 05 '19 11:06 mpadge

OK that is rather neat!

Am I correct in that we are looking for something like this to identify say x and y cols:

  • https://github.com/ATFutures/dodgr/blob/master/R/graph-functions-misc.R#L84 and
  • https://github.com/ATFutures/dodgr/blob/master/R/graph-functions.R#L43

njtierney avatar Jun 05 '19 13:06 njtierney

yeah, the details are all pretty easy. The difficult and important thing was figuring out that the get_columns function should return a list. From that point on, it's all easy

mpadge avatar Jun 05 '19 15:06 mpadge