Skip to content

Moving forward with IsaricDraw #9

@alasdairwilson

Description

@alasdairwilson

Right now the draw layer is still a bit....yeuch: we pass fig_name around, import by convention like we assume that fig name exists in isaricdraw, and call whatever matches. That works, but it gets fragile when I want to move drawing into a separate package or let users override specific plots cleanly. The api is implicit, dispatch is scattered, and failure handling can easily leak into app behavior instead of being contained.

I already made some progress on fixing this at the panel level with StaticInsightPanel: it acts as an adapter/fake class that preserves the same create_visuals call signature while returning prebuilt visuals. I went with this purely to marry the publioc and analysis dashboards into one thing but it also keeps the app-side api stable whether plots are being freshly constructed or loaded from static outputs.

I have two potential design choices that I think work:

  1. subclass walking

This approach discovers available draw classes at runtime by walking BaseDraw.subclasses(), then selecting the first class that says it can handle a fig_name. It reduces explicit wiring, but only works for classes that have already been imported.

So we would do:

class BaseDraw:
    fig_names = ()
    @classmethod
    def can_draw_fig(cls, fig_name): return fig_name in cls.fig_names

def all_subclasses(cls):
    out = set(cls.__subclasses__())
    for sub in list(out): out |= all_subclasses(sub)
    return out

def resolve_draw(fig_name):
    return next((c for c in all_subclasses(BaseDraw) if c.can_draw_fig(fig_name)), None)

This is a design that I have used before but I dont think it quite fits, the reason being that there is no real incentive for you to not know in advance what figure type you want, the way I have used this in the past is if you have metadata that is FIXED and you want to write a class that is appropriate for certain combinations of meta data, e.g. if youw anted to say bar charts but not with log scales and not if you have more than 50 data points.

so instead I suggest:

  1. Explicit registration
REGISTRY = {}

def register_draw(cls):
    for name in cls.fig_names: REGISTRY[name] = cls
    return cls

class BaseIsaricDraw: 
	fig_names = ()

@register_draw
class BarChartDraw(BaseDraw):
    fig_names = ("bar_chart", "bar_chart_stacked")

then we would retrieve the class with:

def resolve_draw(fig_name):
    return REGISTRY.get(fig_name)

A metaclass/introspection route (scan subclasses and ask can_draw_fig(fig_name)) is elegant, but it depends on import timing, which can get messy and as I said abnove, the benefits are maybe not here so.... The cleaner option might be explicit self-registration: each draw class declares which fig names it handles and registers itself in a registry at import time. Dispatch becomes deterministic, overrides are predictable (priority or last-registration-wins), and vertex can wrap calls in one safe adapter that logs and skips bad plugins instead of crashing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestrefactorRefactoring changes

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions