biodiv / anycluster

Server-side clustering of map markers for (Geo)Django
MIT License
106 stars 21 forks source link

Ambiguity with filters, array or dict. Suggestion #30

Closed 2trc closed 1 year ago

2trc commented 6 years ago

Hi, I'm really thankful to this project and all the work you've put in. It's surprising there is no alternative to AnyCluster in server-side clustering area in Python/Django. Kudos!

Regarding filtering, I've noticed some ambiguity in the code (JS vs Python) and obviously the docs.

In the method constructFilterstring in MapCluster.py, filters is treated as a dictionary which is in-line with the docs

for column in filters:
            filterparams = filters[column]
...

However, in anycluster.js, filters is treated as if it was an array

addFilters: function(newfilters){

    for (var f=0; f<newfilters.length; f++){

        this.filters.push(newfilters[f]);
    }

    this.clearMarkers = true;

},

removeFilters: function(activefilters){

    for (i=0; i<= activefilters.length; i++){

        delete this.filters[ activefilters[i] ];
    }

    this.clearMarkers = true;

},

I believe this is a problem.

I would actually suggest, if I may, the array approach. It'd enable filtering multiple times on the same column. It is less limiting than the alternative and one use case is when you'll like to filter on a range e.g. a date range. With the current implementation it'd be really hard to achieve without modifying AnyCluster anyways. I've been using this approach in my current project.

What do you think?

biodiv commented 6 years ago

Thank you for your input and your thoughts. I agree that filters should be of the same type in JS and Python if possible. It haven't been into anycluster for a while but currently your suggestion makes sense to me and seems to improve anyclusters functionality. If you already have something that works and uses arrays in python, a pull request would be great. Otherwise I will dig into it when I have the time to do so.

2trc commented 6 years ago

@biodiv done! See https://github.com/biodiv/anycluster/pull/31 It'd be good if you could try it out in your environment

2trc commented 6 years ago

It'd be even more intuitive to move one step further and have the filterObj format like

var filterObj = [
   {"column": db_column_name, "values": value, "operator": operator_string }
]

wouldn't that be cleaner?

biodiv commented 6 years ago

The same layout came to my mind when reviewing the pull request - your suggested layout would be great

biodiv commented 1 year ago

the suggested format is now implemented