anycluster icon indicating copy to clipboard operation
anycluster copied to clipboard

Ambiguity with filters, array or dict. Suggestion

Open 2trc opened this issue 7 years ago • 4 comments

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?

2trc avatar Jan 02 '18 10:01 2trc

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.

biodiv avatar Jan 02 '18 12:01 biodiv

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

2trc avatar Jan 02 '18 13:01 2trc

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?

2trc avatar Jan 02 '18 14:01 2trc

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

biodiv avatar Jan 04 '18 09:01 biodiv

the suggested format is now implemented

biodiv avatar Apr 24 '23 06:04 biodiv