paulopes / panel-components

HTML components for Panel templates
Apache License 2.0
6 stars 1 forks source link

Remove helper functions from public api of Component #11

Open MarcSkovMadsen opened 4 years ago

MarcSkovMadsen commented 4 years ago

When doing tab completion on the Component I get a lot of public functions back. For example I would see the function add_attributes. To me add_attributes is a helper function and a part of the private api.

The consequence of making a lot of functions public are

  1. The user of a custom Component is overwhelmed by functions to choose from.
  2. The user of a custom Component might use them and start depending on them. Making it harder to change the implementation later.

I would like to make it super easy for a user of the FastButton below to see what he can do with it.

Example

import param
from panel_components.component import Component

class FastButton(Component, param.Parameterized):
    clicks = param.Number()

    def __init__(self, *children, **attributes):
        self._children = children
        self._attributes = attributes
        super().__init__(
            *children,
            tag_name="fast-button",
            opening="<",
            closing="/>",
            main=None,
            css_classes=None,
            **attributes
        )

def test_fast_button():
    fast_button = FastButton("Click Me", appearance="stealth")

    assert fast_button.clicks == 0
    assert callable(fast_button.servable)

image

I would like the user to not see a lot more than the clicks attribute.

Solution

Hide a lot of functions from the public api by putting an underscore _ in front of the function name.

MarcSkovMadsen commented 4 years ago

Complication

There are two types of users.

The user who implements custom components currently uses some of the functionality in the public api. For example prepend_body_css etc. like below.

def page(*children, **attributes):
    if IS_A_JUPYTER_NOTEBOOK:
        component = vue("", *children, style="height: 80vh", **attributes)
    else:
        component = vue("", *children, **attributes)
    (
        component.asset_folders(
            os.path.join(os.path.dirname(os.path.abspath(__file__)), "www")
        )
        .prepend_body_css(bootstrap="bootstrap/bootstrap.min.css")
        .prepend_body_css(
            typography="typography/core.min.css",
            bootstrap_vue="vue/bootstrap-vue.min.css",
        )
        .append_body_js(
            bootstrap_vue="vue/bootstrap-vue.min.js",
            bootstrap_vue_icons="vue/bootstrap-vue-icons.min.js",
            portal_vue="vue/portal-vue.umd.min.js",
        )
        .append_body_script(
            bootstrap_vue="Vue.use(BootstrapVue);Vue.use(BootstrapVueIcons)"
        )
    )
    return component

I would suggest to support specifying the resources in the constructor instead of using methods after construction. For example like

def page(*children, **attributes):
    if IS_A_JUPYTER_NOTEBOOK:
        style="height: 80vh"
    else:
        style=None

    asset_folders = [os.path.join(os.path.dirname(os.path.abspath(__file__)), "www")]
    prepend_body_css = dict(
        bootstrap="bootstrap/bootstrap.min.css",
        typography="typography/core.min.css",
        bootstrap_vue="vue/bootstrap-vue.min.css",
    )
    append_body_css = dict(
        bootstrap_vue="vue/bootstrap-vue.min.js",
        bootstrap_vue_icons="vue/bootstrap-vue-icons.min.js",
        portal_vue="vue/portal-vue.umd.min.js",
    )
    append_body_script = dict(bootstrap_vue="Vue.use(BootstrapVue);Vue.use(BootstrapVueIcons)")

    return vue(
        "",
        *children,
        asset_folders=asset_folders,
        prepend_body_css=prepend_body_css,
        append_body_css=append_body_css,
        append_body_script=append_body_script,
        style=style,
        **attributes
    )
MarcSkovMadsen commented 4 years ago

Alternative Suggestion: ComponentResources class

In order to simplify the api that meets a "simple" user a lot of the resources settings could be moved into ComponentResources class or similar.

That would enable creating "simple" Component.__init__ for the simple user. But with a lot of functionality for the power user. The power user can do something like

def page(*children, **attributes):
    if IS_A_JUPYTER_NOTEBOOK:
        style="height: 80vh"
    else:
        style=None

    resources = ComponentResources()
    resources.folders.assets.append(os.path.join(os.path.dirname(os.path.abspath(__file__)), "www"))
    resources.body.prepend.css_files = dict(
        bootstrap="bootstrap/bootstrap.min.css",
        typography="typography/core.min.css",
        bootstrap_vue="vue/bootstrap-vue.min.css",
    )
    resources.body.append.css_files = dict(
        bootstrap_vue="vue/bootstrap-vue.min.js",
        bootstrap_vue_icons="vue/bootstrap-vue-icons.min.js",
        portal_vue="vue/portal-vue.umd.min.js",
    )
    resources.body.append.js_raw=dict(bootstrap_vue="Vue.use(BootstrapVue);Vue.use(BootstrapVueIcons)")

    return vue(
        "",
        *children,
        resources=resources,
        style=style,
        **attributes
    )

Also the tab completion would be nice as you don't get a short list of options than if you put all the attributes on the Component it self.

Also a ComponentResources instance can be shared across components if needed.

A downside of moving the resources out of the Component.__init__ function and into a nested object is that docstrings are not available.

Additional Context

Maybe it's better to use the terms top and bottom instead of append and prepend to distinguish it from something you do to a list.

Implementation

A part of the implementation could look like

from typing import Optional, Dict, List

class FolderResources():
    def __init__(self, src="www", dst="static", assets: Optional[List[str]]=None):
        self.src: str = src
        self.dst: str = dst
        self.assets: List[str] = assets or []

class HeadResources():
    pass

class BodyAppendResources():
    pass

class BodyPrependResources():
    def __init__(self, css_files: Optional[Dict]=None):
        self.css_files: Dict[str, str] = css_files or {}

class BodyResources():
    def __init__(self, append: Optional[BodyAppendResources]=None, prepend: Optional[BodyPrependResources]=None):
        self.append = append or BodyAppendResources()
        self.prepend = prepend or BodyPrependResources()

class ComponentResources():
    def __init__(self):
        self.folders = FolderResources()
        self.head = HeadResources()
        self.body = BodyResources()
paulopes commented 4 years ago

Another alternative might be not to subclass from Component, and just create your instance of Component in the constructor, that way only exposing in your API what you want to be exposed to the user.

MarcSkovMadsen commented 4 years ago

But for my usecases i would like to create Components with Parameters and bidirectional communication like

class FastButton(Component, param.Parameterized):
   clicks=param.Integer()