Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for pre- and post-processing hooks on Operation #2246

Closed
philippjfr opened this issue Jan 6, 2018 · 1 comment
Closed

Add support for pre- and post-processing hooks on Operation #2246

philippjfr opened this issue Jan 6, 2018 · 1 comment

Comments

@philippjfr
Copy link
Member

philippjfr commented Jan 6, 2018

Operations are a very powerful concept and we ship a variety of operations that are very useful in a lot of contexts. This is particularly true of the datashader operations but also of things like bivariate and contours operations which can be useful for geographic plotting. However to add support for geographic elements and coordinate systems to these operations they would have to be subclassed, which would require users to use the appropriate operation adding extra complexity. In order to extend these operations from external libraries like GeoViews I'd like to propose adding preprocess_hooks and postprocess_hooks class attributes to the Operation baseclass.

The implementation could look something like this:

def _apply(self, element, key=None):
    kwargs = {}
    for hook in self.preprocess_hooks:
        kwargs.update(hook(element))
    ret = self._process(element, key)
    for hook in self.postprocess_hooks:
        ret = hook(ret, kwargs)
    return ret

GeoViews could then register hooks like this to grab the crs and make sure it is added to the returned element:

def convert_to_geotype(element, crs=None):
    geotype = getattr(gv, type(element).__name__, None)
    if crs is None or geotype is None:
        return element
    return geotype(element, crs=crs)

def get_crs(element):
    crss = element.traverse(lambda x: x.crs, [gv.element._Element])
    crss = [crs for crs in crss if crs is not None]
    if any(crss[0] != crs for crs in crss[1:] if crs is not None):
        raise ValueError('Cannot datashade Elements in different '
                         'coordinate reference systems.')
    return {'crs': crss[0]}

def add_crs(element, kwargs):
    return element.map(lambda x: convert_to_geotype(x, kwargs.get('crs')), hv.Element)
    
datashade.preprocess_hooks.append(get_crs)
datashade.postprocess_hooks.append(add_crs)

Alternatively instead of having an _apply method that passes the outputs of the preprocessors to the postprocessors the crs could be stored in the form of some metadata on the operation instance.

This proposal will address holoviz/geoviews#45.

@jlstevens
Copy link
Contributor

This seems like a reasonable proposal.

My only comment is that I don't think users should be using such a mechanism and this should be for library authors extending holoviews (geoviews is a good example). You don't want to reason about what user added hooks may or may not have been added to understand what an operation is doing.

For this reason, I would make these class attributes into _preprocess_hooks and _postprocess_hooks so geoviews can use them and maybe advanced users who really know what they are doing, though really they should be warned off by the underscore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants