Fixes to scalebar() - Githubissues

oswaldosantos / ggsn

R package to add north symbols and scale bars to maps created with ggplot or ggmap

http://oswaldosantos.github.io/ggsn/

GNU General Public License v2.0

161 stars 8 forks source link

Fixes to scalebar() #49

Closed DesiQuintans closed 5 years ago

DesiQuintans commented 5 years ago

scalebar: 'data' arg now checks that 'lat' and 'long' columns exist, and the user is told of this requirement.
scalebar: 'data' arg now only keeps columns that begin with 'lat' or 'long' Users are warned not to have other columns that meet this condition.
scalebar: function now stops the user if they are asking for a scalebar that is too big to fit on the map.

I have not edited the DESCRIPTION, I leave it to you if you accept the PR. Cheers!

DesiQuintans commented 5 years ago

Will change the wording.

Re. equal names, it's up to you. $ does partial matching of names so I think it's quite convenient that it can match both lat and latitude, because I've used datasets that use either of those and it saves the user the trouble of renaming from latitude. A friend of mine also ran into issues using x and y, which is what #46 was inspired by.

oswaldosantos commented 5 years ago

Ok, the partial matching is fine. Just remove the code to drop the columns other than latitude and longitude to accept your pull request, it has a bug #47.

DesiQuintans commented 5 years ago

Re. #47, instead of asking for 4 different args (x.min/x.max/y.min/y.max), what about asking for only x and y as vectors and calculating it for the user? Unsure if you'd want to create two new x/y args for this case to maintain backwards-compatibility, or if you'd prefer to detect missing arguments or certain kinds of input (e.g. if user provides x.min and y.min as numeric vectors with length > 1, then do it for them. Maybe this is splitting hairs and out of the scope of the pull request.