Attribution for code from {snapbox}?

[MilesMcBain]

Hi @walkerke and @elipousson,

I appreciate the attribution @anthonynorth and I received in this tweet for code taken from {snapbox}: https://twitter.com/elipousson/status/1513962285059956746

There is no attribution in this package though, is that right?

I don't know about you, but I try to take the attribution requirement of the MIT license seriously. Free use with attribution is a different thing to free use.

Here is an example of how I have done this myself, in an R package:

• In file attribution: https://github.com/MilesMcBain/wisegroup/blob/master/tests/testthat/test-mutate....R#L1
• Add copyright holders in DESCRIPTION: https://github.com/MilesMcBain/wisegroup/blob/master/DESCRIPTION#L14-L19

I'm not a lawyer, so I don't know if this perfectly meets the legal requirements, but certainly no one can deny an effort was made to comply with the license. It was also based on advice from Hadley Wickham as to the steps that needed to be taken.

Do with this information what you will, I'm not going to make a big deal about this if you don't attribute us, but it would be appreciated if you did.

[walkerke]

Thanks @MilesMcBain! I've added both you and Anthony as contributors and copyright holders in the package DESCRIPTION and your authorship was added to layer_static_mapbox(). Please review when you get a minute and let me know if you'd like any further attribution, I'd be happy to. @elipousson would you mind reviewing your contributions and seeing if there are other locations where attribution would be appropriate?

[MilesMcBain]

Thanks @walkerke! I think you only need to add Anthony as copyright holder though, because he is the copyright holder of the code for {snapbox}. I had two copright holders in that example because they were both listed in the LICENSE. {snapbox}'s LICENSE only lists Anthony North.

[elipousson]

Apologies for the oversight on this, @MilesMcBain. We talked about the need for attribution in my original issue for this feature addition but I never followed up to get it into the package.

I believe the only two pieces of code that I incorporated directly from {snapbox} are in the set_static_map_dims helper function and the lonlat_to_worldcoords helper function. A few other elements are inspired by the general approach used by {snapbox} but I believe the rest of the code was either in {mapboxapi} already or is something I added.

[elipousson]

Whoops – after checking again it looks like I didn't end up using lonlat_to_worldcoords (I think I brought it over but then didn't end up needing it). It should probably be removed which would make set_static_map_dims is the only {snapbox} code in the package.

[MilesMcBain]

Thanks @elipousson, no need to apologise, it's clear we're all acting in good faith here.

One other thing I would add is that it's not just code copied verbatim that it makes sense to attribute. If you transcribe something, modifying it in the process, that's still modification as described by the license IMHO.

I could be wrong but it looks like that's what happened with the static map code. While not the same exact code, it contains lines that are exactly the same, or lines that are exactly the same but for variable names changed, and it uses arguments in the API e.g. scale, that are kind of quirky and we pretty much regret.

It's definitely a grey area, so not trying to force anything.

[elipousson]

That is really helpful context, @MilesMcBain – I didn't initially think about those broader factors as something where a license or contributor acknowledgement would be appropriate but it all makes sense. I definitely want {mapboxapi} to credit you and Anthony and to link users back to {snapbox} so they can try it out and compare.

If it is OK with @walkerke, I can create a pull request to do the following:

• Remove you as a copyright holder from the DESCRIPTION but keep Anthony as a copyright holder (and keep you both as contributors)
• Update the description of layer_static_mapbox to explain that the function uses code from and is also based on the layer_mapbox function from {snapbox}

Is there anything else you think we should update or add?

Also – this may be a better topic for the #17 issue but, since this function is a new addition to {mapboxapi} there is probably still time to update function argument names. Do you have an alternative term for scale that you think may be better to use instead? I kept the argument names the same intentionally to give a similar user experience as {snapbox} (not quite realizing it was an intellectual property issue) but would be happy to change it.

[walkerke]

Yes, that PR sounds just fine, @elipousson.

Regarding the argument names; most of it uses argument names I originally wrote for static_mapbox(), correct? I wrote that function independent of the snapbox package so any other argument similarity is likely due to the fact that both functions hit the same API endpoint. Most of those arguments are passed directly to the API (https://docs.mapbox.com/api/maps/static-images/) so we probably wouldn't want to change them.

[elipousson]

You're correct about the argument names, @walkerke . There are a couple of differences/similarities so I just wrote up some notes (not because I think it is needed for attribution but just in case the clarification is helpful in the future):

• For the API parameter "@2x", {mapboxapi} uses "scale_factor" and {snapbox} uses "retina" (boolean to switch from 1x to 2x)
• For the API parameter "logo", {mapboxapi} uses "logo" and {snapbox} uses "mapbox_logo"
• For the API parameter "attribution", {mapboxapi} uses "attribution" and {snapbox} uses "attribution"
• The {snapbox} "scale_ratio" parameter (used to determine map pixel dimensions) is implemented in {mapboxapi} as "scale" (I copied the parameter definition directly)
• The {snapbox} "map_style" parameter is effectively implemented in {mapboxapi} using the "style_url" parameter. The API uses the "username" and "style_id" parameters which also are used by {mapboxapi} but not {snapbox}
• The {snapbox} parameter "area" already had an equivalent parameter in {mapboxapi} named "location" (I dropped a previously supported bbox parameter when I made these changes and added bbox support to the location parameter)

The only parameter name that may be worth considering for a change is "scale" which doesn't have an exact equivalent in the Mapbox API and was only added on March 27 so could likely still be changed with minimal disruption for users. Of course, I only brought it up because it sounds like @MilesMcBain may have a better idea for that name (or at least think scale may not be the best name to use).

[elipousson]

One more quick question: do you mind @walkerke if I add a global "@md" tag to the DESCRIPTION to support Markdown in the function descriptions. I can also add in more links to the Mapbox API documentation while I'm making these updates. I also would love to add a couple tests if that is OK.