flekschas / regl-scatterplot

Scalable WebGL-based scatter plot library build with Regl
https://flekschas.github.io/regl-scatterplot/
MIT License
185 stars 21 forks source link

Unable to filter by empty array #122

Closed insertmike closed 1 year ago

insertmike commented 1 year ago

Abstract

filter event does not allow hiding all points:

  /**
   * Filter down to a set of points
   * @param {number | number[]} pointIdxs
   * @param {import('./types').ScatterplotMethodOptions['filter']}
   */
  const filter = (pointIdxs, { preventEvent = false } = {}) => {

     filteredPoints = Array.isArray(pointIdxs) ? pointIdxs : [pointIdxs];

    if (filteredPoints.length === 0) return unfilter({ preventEvent }); // <----

I don't see this as intended behavior. When user tries to filter by empty array, regl-scatter calls the unfilter event under the hood and this causes inconsistent behavior. If this is intended, at least it should be stated in the library documentation.

Proposal

  1. Allow filtering by removing the .unfilter line from the filter handler above.
  2. Adjust all other related code depending on this behavior (e.g):
  const getPoints = () => {
    if (filteredPointsSet.size > 0) //  <--
      return searchIndex.points.filter((_, i) => filteredPointsSet.has(i));
    return searchIndex.points;
  };

  const getPointsInBBox = (x0, y0, x1, y1) => {
    const pointsInBBox = searchIndex.range(x0, y0, x1, y1);
    if (filteredPointsSet.size > 0) // <--
      return pointsInBBox.filter((i) => filteredPointsSet.has(i));
    return pointsInBBox;
  };
  1. Update tests and documentation.

@flekschas I can take lead on this just let me know your thoughts first. And if there is anything else related that I am not aware of, connected to this behavior. It is all about how much time it will cost.

flekschas commented 1 year ago

I agree, this behavior is inconsistent and we can allow filtering out all points.

As you take a stab at this, please be aware that if (filteredPointsSet.size > 0 was added because filteredPointsSet is initialized as an empty set. See https://github.com/flekschas/regl-scatterplot/blob/master/src/index.js#L289. We likely have to update filteredPointsSet in setPoints() (https://github.com/flekschas/regl-scatterplot/blob/master/src/index.js#L1753) accordingly, otherwise getPoints() will be broken.