mapnik / Cascadenik

Cascading Sheets Of Style for Mapnik
http://github.com/mapnik/Cascadenik/wiki/Cascadenik
BSD 3-Clause "New" or "Revised" License
111 stars 20 forks source link

Map attributes parsing #9

Open springmeyer opened 13 years ago

springmeyer commented 13 years ago

We should respect map level attributes like 'min_version' when an mml parsed by python, and stuff like 'font_directory' which will be available in mapnik2.

I'll take this on, but it can happen after initial release.

gravitystorm commented 13 years ago

Making this issue easier to find: cascadenik support for "minimum_version"

migurski commented 12 years ago

Is this still a thing? Did "font_directory" get added?

springmeyer commented 12 years ago

Yes, it did. And before the 2.0.0 release all parameters were made to use consistent dashes (instead of underscores), so its now font-directory, and it can be passed a directory or a single file:

https://github.com/mapnik/mapnik/blob/master/tests/data/good_maps/extra_known_map_attributes.xml

mojodna commented 10 years ago

So, I'm trying to fix this because I'm off in the weeds on a Friday evening...

I've gotten as far as getting font-directory to pass through as a URI: mojodna/Cascadenik@b4e1385516954746f30555c06b6402d7cf31f63a

However, I'm having trouble associating it with a mapnik.Map here: https://github.com/mapnik/Cascadenik/blob/82f66859340a31dfcb24af127274f262d4f3ad85/cascadenik/output.py#L32

It's an "extra param" (i.e. not directly part of the public API, along with minimum_version per extra_known_map_attributes.xml), so it should be attached to mmap.parameters, which is a mapnik._mapnik.Parameters instance and appears to be read-only, but I'm probably missing something.

Ideas?

mojodna commented 10 years ago
mmap.parameters.append(mapnik._mapnik.Parameter("font-directory", "fonts/"))
mapnik.save_map(mmap, "style.xml")

outputs:

<?xml version="1.0" encoding="utf-8"?>
<Map srs="+proj=longlat +ellps=WGS84 +datum=WGS84 +no_defs">
    <Parameters>
        <Parameter name="font-directory" type="string">fonts/</Parameter>
    </Parameters>
</Map>

This what I was expecting (https://github.com/mapnik/mapnik/blob/master/tests/data/good_maps/extra_known_map_attributes.xml, effectively):

<Map srs="+proj=longlat +ellps=WGS84 +datum=WGS84 +no_defs" font-directory="fonts/" />

@springmeyer are these intended to be interchangeable? https://github.com/mapnik/mapnik/blob/49feda9b13e6db416647afad8204b8d7d6c22024/src/load_map.cpp#L263 makes me think that they're not (since it's reading font-directory from the <Map> node).

Is this a bug in the Python bindings?

springmeyer commented 10 years ago

Yes, a bug in the bindings resulting from the oddity of this parameter. Basically, font-directory only matters at the time the xml is loaded so it did not make sense to add it as a new property. So, I think my reasoning was to push it into the extra attributes so that it would not get lost and could be serialized still. But it has gotten lost.

springmeyer commented 10 years ago

hmm, forgot that I actually did away with this support (https://github.com/mapnik/mapnik/commit/ea5a46f2302e7706149c2bef91dc48d06163e71c#diff-bc721da090a4fdd48972b8aad7f14ec8). So, font-directory is not able to be round tripped and there is no way to add it via python.

mojodna commented 10 years ago

Well, crap. Thanks for looking into it.

springmeyer commented 10 years ago

it this lack of round tripping a major problem?

mojodna commented 10 years ago

No, I think I've got a better way of dealing with it that doesn't involve Cascadenik.

springmeyer commented 10 years ago

great. so I'll close this. Ping back if you have reservations. I'm planning on reworking the behavior of font-directory in Mapnik master soon (consider making it apply fonts locally to map rather than to global cache) - in this case it would again be round-trippable.

springmeyer commented 10 years ago

note: font-directory round tripping will work around at Mapnik 2.3.x

mojodna commented 10 years ago

Awww.. That was very kind of you--thanks! Reopening since it's fixable now!