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

Define properties in a more declarative manner? #362

Closed
almarklein opened this issue Feb 22, 2017 · 9 comments · Fixed by #408
Closed

Define properties in a more declarative manner? #362

almarklein opened this issue Feb 22, 2017 · 9 comments · Fixed by #408

Comments

@almarklein
Copy link
Member

almarklein commented Feb 22, 2017

Properties are now defined using:

@event.prop
def foo(self, v=0):
    return int(v)

Tools like Bokeh, Traits, Traitlets and others adopt a more declarative style:

foo = event.Int(0)

The latter is obviously shorter, and makes things easier. This approach should also be possible with Flexx, but I've never implemented this, mostly because:

  • If you add a sizable docstring, the former approach becomes the more natural variant, IMO.
  • Some properties that allow different kinds of values become rather ugly. The imperative approach allows any flexibility that you need.
  • I feel strongly for a "there should be exactly one way to define a property" approach.

That said, most properties are simple and have a short docstring. So let's consider adopting the declarative approach. Maybe we can find a way to define custom property classes to allow flexible values, but still have the prop itself be defined in the same declarative way.

I remember that in the very early stages of Flexx I used a declarative approach and I ended up writing property classes for parent and children, but maybe that was just because at that time the serializer was not yet able to handle Model objects.

This is a continuation of a side-discussion in #359. cc @Korijn @frmdstryr

@almarklein
Copy link
Member Author

Here is an example from Bokeh which made me lean towards an imperative system:

bounds = Either(
        Auto,
        Tuple(Float, Float),
        Tuple(Datetime, Datetime),
        Tuple(Int, Int),
        help="""
    The bounds that the range is allowed to go to - typically used to prevent
    the user from panning/zooming/etc away from the data.
    
    By default, the bounds will be computed to the start and end of the Range.
    
    Bounds are provided as a tuple of ``(min, max)`` so regardless of whether your range is
    increasing or decreasing, the first item should be the minimum value of the range and the
    second item should be the maximum. Setting min > max will result in a ``ValueError``.
    
    Setting bounds to ``None`` will allow your plot to pan/zoom as far as you want. If you only
    want to constrain one end of the plot, you can set min or max to None.
    
    Examples:
    
        Range1d(0, 1)  # Increasing range, auto-bounded to 0 and 1 (Default behavior)
        Range1d(0, 1, bounds=(-0.1, 1.1))  # Increasing range with bounds at -0.1 and 1.1
        Range1d(1, 0, bounds=(-0.1, 1.1))  # Decreasing range with bounds at -0.1 and 1.1
        Range1d(0, 1, bounds=(0, None))  # Increasing range bounded at minimum of 0, unbounded maximum
        Range1d(start=0, end=1, bounds=None)  # Unbounded range
    """)

@almarklein almarklein changed the title Define properties in a more declarative manner Define properties in a more declarative manner? Feb 22, 2017
@frmdstryr
Copy link

Can you also show a potential imperative implementation of the above for comparison?

@almarklein
Copy link
Member Author

Very good point. Something like this. I must admit that it does not look that much better now that I see it. Maybe I have a tendency towards imperative code for some reason. Not sure if that's because its generally easier to follow, or whether its just me.

@event.prop
def bounds(self, v='auto'):
    """ The bounds that the range is allowed to go to - typically used to prevent
    the user from panning/zooming/etc away from the data.
    
    By default, the bounds will be computed to the start and end of the Range.
    
    Bounds are provided as a tuple of ``(min, max)`` so regardless of whether your range is
    increasing or decreasing, the first item should be the minimum value of the range and the
    second item should be the maximum. Setting min > max will result in a ``ValueError``.
    
    Setting bounds to ``None`` will allow your plot to pan/zoom as far as you want. If you only
    want to constrain one end of the plot, you can set min or max to None.
    
    Examples:
    
        Range1d(0, 1)  # Increasing range, auto-bounded to 0 and 1 (Default behavior)
        Range1d(0, 1, bounds=(-0.1, 1.1))  # Increasing range with bounds at -0.1 and 1.1
        Range1d(1, 0, bounds=(-0.1, 1.1))  # Decreasing range with bounds at -0.1 and 1.1
        Range1d(0, 1, bounds=(0, None))  # Increasing range bounded at minimum of 0, unbounded maximum
        Range1d(start=0, end=1, bounds=None)  # Unbounded range
    """
    if v == 'auto' or v is None:
        return None
    elif isinstance(v, tuple) and len(v) == 2:
        for el in v:
              if not (el is None or isinstance(el, (float, int, Datetime))):
                    raise TypeError('bounds should be int, float, Datetime or None')
        return v[0], v[1]
    else:
        raise TypeError('Bounds should be "auto" or a tuple of int/float/Datetime')

@Korijn
Copy link

Korijn commented Feb 22, 2017

I guess I totally misinterpreted the discussion. In my opinion, we're not talking about declarative/imperative here, but about whether or not you do type checking yourself. The properties are still "declared" in both examples.

What I care about is how properties are connected, and how to cope with computed properties. It doesn't seem like any of that hinges on the property declaration "style".

@Korijn
Copy link

Korijn commented Feb 22, 2017

On topic, though, I would prefer not having validation at all. Just a property with a default value is fine. I don't want to "autocorrect" values when they are e.g. flat instead of nested, or the other way around. My application should just use one consistent format instead! In my experience, autocorrecting behaviour can be very misleading and hide bugs.

What is your take?

@almarklein
Copy link
Member Author

I think that validation (and normalization) of properties can be convenient, and help make your app stable, especially if properties are set/get in very different parts of an app. It is much less useful if a property is only get/set from within the same class, or if a property is set from a single place (like readonlies typically are), where you put an effort in setting it correctly.

This discussion changes somewhat in light of some new ideas which I'll post in a new issue soon.

@Korijn
Copy link

Korijn commented Feb 24, 2017

I'm not sure if this is the right place to put my thoughts then, but here's what I would appreciate:

class MyWidget(ui.Widget):
    counter = event.Property(0)
    todos = event.Property([])

Kind of like a mix between the two examples at the top of this thread. It's a class attribute, but it isn't typed. It's just an evented property with a default value.

@almarklein
Copy link
Member Author

I think Bokeh and the likes call this Any() or Object().

@frmdstryr
Copy link

Atom uses Value(). It's pretty much the base property without any type checking/validation. Other properties then extend it to add those.

Atom also has Property(_getter_fn(),_setter_fn()) which can also be cached.

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

Successfully merging a pull request may close this issue.

3 participants