Filters

[fdansv]

No description provided.

[alonsogarciapablo]

I don't understand why the Filter is part of the renderer, and why is renderer responsible of adding tiles to the Filter. I'd expect someone listening to the "tileAdded" event to load the data into an object where you can add filters.

[rochoa]

For me it's a bit strange as well.

[fdansv]

The layer (CartoDBd3Layer) receives a geojson tile that contains/can contain several layers, each one of which is represented by a renderer. Keeping that in mind, would it be better to create a new class, called something along the lines of sublayer, and restrict the functions of the renderer to generating the svgs?

[alonsogarciapablo]

I've been looking into this a bit and I still don't quite get the concept of Filter and Expressions. Perhaps it's because of the naming...

I would expect to have an object where you could add filters and query for the stuff we need for widgets. Here's some pseudocode (I called this object datasource but it could be whatever makes sense)

// { city: 'Madrid', population: '10' }
// { city: 'Barcelona', population: '20' }
// { city: 'A Coruña', population: '30' }

var categoryFilter = new CategoryFilter({
  widgetId: 'myWidget1', // not sure if this is needed
  column: 'city'
});
var rangeFilter = new RangeFilter({
  widgetId: 'myWidget2', // not sure if this is needed
  column: 'population',
  bins: 10
});

datasource.addFilter(rangeFilter);
datasource.addFilter(categoryFilter);

datasource.getCategories('myWidget1');
// It would return all cities

// Change the filter
rangeFilter.setRange(20, 30);

datasource.getCategories(); // not sure if it would need more paremeters like the column or widgetId
// It would return "Barcelona" and "A Coruña"

categoryFilter.reject("Barcelona")

datasource.getCategories(); // not sure if it would need more paremeters like the column or widgetId
// It would return "A Coruña"

datasource.getHistogram();

[fdansv]

@alonsogarciapablo right now I'm focusing more on the logic of the widgets themselves, but here's the way I'm seeing it:

• First of all, I called expressions to the declarations of widgets within the filtering system. It's probably a poor choice of naming and I'm open to changing it, or just call them widgets as well. For the sake of clarity I'll refer to them here as expressions.
• When we add a new expression, we're asking the Filter to "keep track" of the column, with the parameters indicated. The main reason why we do this is that we have to instantiate a dimension with it. Dimensions are super expensive to create and store, so we can't just create new ones as the widgets ask for data.

filter.addExpression("category_de_pepito", {
  type: "aggregation",
  column: "sov_a3",
  aggregation: "count",
  sync: true
});

• When we keep track of that column with those parameters, this means that their returning data will be recalculated each time filter.update() is run. What that method does is iterating over the list of expressions that have been declared and run the function that was generated for each one at the moment of the declaration (see this, for example).

filter.update();
// will update the this.report object, and return:

{
  "category_de_pepito": {
    "count": 2722,
    "nulls": 0,
    "min": 1,
    "max": 145,
    "categoriesCount": 136,
    "categories": [
      {
        "category": "RUS",
        "value": 145,
        "agg": false
      }, // [...]
    ],
    "type": "aggregation"
  }
}

• As soon as I'm done coding histograms I'll add both a method to filter according the widget id and a getter function for the widgets to use.

[javisantana]

I think the abstraction level for that api is higher than expected. I'd expect something like:

for filtering:

layer.filterRange('attr', [0, 1000]);
layer.filterExact('attr2', 'pepe');
layer.clearFilters();

and to fetch data:

var values = layer.getValues('attr') // values will contain the attr values with the current filters applied
// histogram other stuff is calculated outside the layer

[fdansv]

OK so now the Filter is connected to the rendering part so setting a filter with the API above will reflect those changes on the map, which will update when renderer.redraw() is called. Also I added a few tests to provide good coverage for filters.

@rochoa @alonsogarciapablo this is ready to merge my side.

[rochoa]

Set is a ES6 feature. We need another approach or a polyfill.

[fdansv]

👌 just changed this, it was supposed to be a temporary use. Replaced it with a normal JS object.

[rochoa]

As per previous comments I was expecting to have the filtering at layer level, but probably I'm missing something here.

Having some kind of integration test would help a lot to understand the layer high level API.

[rochoa]

Why don't we have this in the constructor? Is there any other way to use initialize apart from being called from the constructor?

[rochoa]

All filter* functions are quite similar. Wouldn't be better to have a generic filter function that receives the actual filter function and applies it by column?

Or even better, having different filter entities with a common interface that can be applied. Having different functions for different types of filters means that we need as many public functions as filters we want to have.

[rochoa]

I'm still missing some tests or examples displaying how the layer/high level API is.