Add copyright/attribution message to downloaded map images #735
Conversation
|
So regarding the temporary file vs. BytesIO topic. I looked at both options and thought it might be safer to store intermediate results on disk, rather than trying to juggle everything in memory. I also noticed the "resource.setrlimit" at the beginning and thought that memory usage is something to watch out for. Are temporary files too much of a concern? |
|
The memory limit is mostly just a backstop against people doing stupid things, especially with the vector output formats as I recall. Temporary files generally make me nervous because you need to be sure they will always get cleaned up properly, especially if like here you're not immediately deleting them, which you can't if you need to be able to pass them to functions by name. Though all the functions here could actually take a handle instead I think. |
Add webp image format. Depends on python3-pil. Fixes openstreetmap/openstreetmap-website#551
mmd-osm
force-pushed
the
feature/exportattribution
branch
from
f796226 to
4b15ef9
Compare
I suspect this is because Mapnik can get memory crazy with some output formats. The memory limit is 4GB, so I don't see that storing the rasterized output in-memory is an issue. PDF and PS are terrible to use with Mapnik and likely to break stuff in other ways, so I wouldn't worry about those. |
| @@ -162,6 +162,7 @@ | |||
| python3-pyotp | |||
| python3-pyproj | |||
| python3-setuptools | |||
| python3-pil | |||
There was a problem hiding this comment.
Can we keep this list alphabetically ordered please?
|
|
||
| if format == "png": | ||
| output_io.seek(0) | ||
| return output_io.read() |
There was a problem hiding this comment.
Why are we reading in the data rather than just returning the handle and letting the called use output_file on it? If you do that you don't need the seek either as output_file already does that.
There was a problem hiding this comment.
The current raster code doesn’t use output_file either (but sys.stdout.buffer.write(image)). I wanted to keep the current code as is. Using output_code would require some attribution dependent logic in line 135. I’m not sure this is better.
| image = mapnik.Image(map.width, map.height) | ||
| mapnik.render(map, image) | ||
|
|
||
| if attribution == 'X': |
There was a problem hiding this comment.
Why are we making the attribution optional?
There was a problem hiding this comment.
Let's suppose I want to put a static map image on my website. If the attribution is optional, I might export the image without an attribution, put it on my site and add a link to osm under it. If the attribution is not optional, I'll see that osm is already credited and I won't add a link.
There was a problem hiding this comment.
That’s what the original issue openstreetmap/openstreetmap-website#551 was proposing. Maybe @simonpoole still remembers…
There was a problem hiding this comment.
At the time my thoughts were that somebody putting the image on a website has ample ways of providing attribution, but somebody that was just printing it out would have to jump through hoops to add it.
But given that the audience is likely getting more unsophisticated by the day it is probably OK to simply always add it.
There was a problem hiding this comment.
At least in openstreetmap/openstreetmap-website#5607 it’s enabled by default and users need to explicitly disable it.
This PR introduces a new URL parameter attribution=X which adds OSM attribution to the map export, both for vector and raster formats. Besides, this PR adds webp image format.
I introduced a new dependency on python3-pil package to enable the png->{webp,jpeg} conversion in "show attribution" mode. This might require further adjustments to https://github.com/openstreetmap/chef/blob/master/cookbooks/tile/recipes/default.rb#L102-L105, which aren't covered in this PR.
If attribution hasn't been requested, the code should behave exactly as before.
Fixes openstreetmap/openstreetmap-website#551