deck.gl icon indicating copy to clipboard operation
deck.gl copied to clipboard
verified publisher icon visgl

GPU Aggregation (8/8): Remove legacy aggregator

Open Pessimistress opened this issue 1 year ago • 4 comments

For https://github.com/visgl/deck.gl/issues/7457

For discussion: there is no more CPUGridLayer/GPUGridLayer, simply GridLayer. Should we keep the old layer exports with the gpuAggregation flag preset?

This affects the carto module @donmccurdy @felixpalmer

Change List

  • Move legacy AggregationLayer into the heatmap-layer directory. HeatmapLayer is the only remaining layer using it, and we are planning a rewrite.
  • Remove unused code and tests

Pessimistress avatar Aug 23 '24 16:08 Pessimistress

Coverage Status

coverage: 90.933% (+1.6%) from 89.292% when pulling b94834016213fdde7846c7cabdbba330d11028cc on x/aggregation-8 into 0dd47b4a2cd930c46064cc8361c2ad05dc2eb464 on master.

coveralls avatar Aug 23 '24 23:08 coveralls

If GPUGridLayer has never worked in the v9.x lifecycle then perhaps that isn't needed? But keeping CPUGridLayer as an alias of GridLayer would be a nice backward-compatibility approach I think.

donmccurdy avatar Aug 26 '24 13:08 donmccurdy

For discussion: there is no more CPUGridLayer/GPUGridLayer, simply GridLayer. Should we keep the old layer exports with the gpuAggregation flag preset?

I also think removing it seems reasonable. Probably not the most common layer, and easy enough for affected users to update.

ibgreen avatar Aug 26 '24 13:08 ibgreen

I think it is reasonable to remove the old layers, it is straightforward for users to rename the layer

felixpalmer avatar Aug 28 '24 07:08 felixpalmer