etowahadams
commented
1 year ago Hi @dvmoritzschoefl,
This is a good point that would help improve adherence to the DRY principal.
Currently datafetchers with workers implementTabularDataFetcher. Instead, these datafetchers could extend a base TabularDataFetcher class which has has a default fetchTilesDebounced function as you suggested.
A little background which might be interesting:fetchTilesDebounced is the the primary way data is passed to GoslingTrack from datafetchers that don't use a worker, but not in datafetchers that use a worker, which instead use getTabularData. So actually, the logic in fetchTilesDebounced could also be removed for classes which implement TabularDataFetcher. However, higlass still expects there to be a function called fetchTilesDebounced so we still include it.
In the long term, we want for Gosling to be able to support custom datafetchers. To do this, we first need to reduce the API surface that is exposed to datafetchers through HiGlass. We are in the process of gradually adding types to HiGlass, which will make it easier to break apart into sub packages which in turn will make it easier to specify what parts of HiGlass the datafetchers have access to.
It seems that currently the debouncing of requests is solved on a per-fetcher basis, meaning that every fetcher needs to implement the same logic again. We did not understand this at the beginning since the fetcher method is called fetchTilesDebounced which suggests that it would not be triggered on each zoom event. Maybe it would be useful to move this code to a base class.