node-mapnik icon indicating copy to clipboard operation
node-mapnik copied to clipboard

Pass variables into grid_renderer

Open rochoa opened this issue 8 years ago • 8 comments
trafficstars

Use a similar approach as in EIO_RenderImage, making variables available at renderer/datasource level.

It should fix #768.

rochoa avatar May 16 '17 14:05 rochoa

Can you please add test cases for this?

flippmoke avatar May 16 '17 14:05 flippmoke

Codecov Report

Merging #769 into master will increase coverage by <.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #769      +/-   ##
==========================================
+ Coverage   96.23%   96.23%   +<.01%     
==========================================
  Files          42       42              
  Lines        8810     8813       +3     
==========================================
+ Hits         8478     8481       +3     
  Misses        332      332
Impacted Files Coverage Δ
src/mapnik_map.cpp 99.92% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c335992...0f64ee5. Read the comment docs.

codecov-io avatar May 16 '17 14:05 codecov-io

@flippmoke, I will be more than happy to add some tests for this one 👍 .

However, I find it a bit difficult as there are no tests for the Postgis input plugin and I don't know about other plugins (I checked shape, geojson, and memory ones) supporting the variables functionality.

How should I proceed? Should I change the CI scripts to install PosgreSQL/Postgis to test it?

Thanks 😄 .

rochoa avatar May 17 '17 08:05 rochoa

@flippmoke, I'm really willing to add the test for this one. How should I proceed?

rochoa avatar May 22 '17 11:05 rochoa

I believe the render tests for grid renderer are located here -- https://github.com/mapnik/node-mapnik/blob/master/test/grid.test.js

It looks like the vector tile renderer already supports this --- https://github.com/mapnik/node-mapnik/blob/master/src/mapnik_vector_tile.cpp#L5174.

So it has some tests here:

https://github.com/mapnik/node-mapnik/blob/master/test/vector-tile.test.js#L2664

Those are probably your best examples.

Could we do this with out the postgis plugin but using another plugin?

flippmoke avatar May 24 '17 14:05 flippmoke

Could we do this with out the postgis plugin but using another plugin?

I think only postgis and pgraster plugins support variables atm.

lightmare avatar May 24 '17 15:05 lightmare

This is a fairly small and safe change that we integrated moths ago into our fork and has been running in our production premises ever since.

Anyway, now we can take advantage of the support for postgis tests added here (thanks!): https://github.com/mapnik/node-mapnik/pull/809

I'll take a look whenever I have a chance to write a few tests for it...

rafatower avatar Jun 15 '18 08:06 rafatower

👍 @rafatower after #884 we're all 🍏 as far as tests. Please create a new PR (or I guess add to this one) when you have tests in place and then I'll merge.

springmeyer avatar Jun 25 '18 17:06 springmeyer