maxfordham / ipyautoui

create ipywidgets user input form pydantic model or jsonschema
https://maxfordham.github.io/ipyautoui/
40 stars 4 forks source link

🐛 setting the `_value` of an `AutoUi`-generated widget results in multiple calls to observers watching `_value` #178

Closed mwcraig closed 1 year ago

mwcraig commented 1 year ago

I was using AutoUi to generate some forms when I encountered unexpected behavior when I observe the _value of a widget: my observer is called multiple times. The number of times is the number of values in the widget that are changing plus 1.

In my particular case, the intermediate calls were causing issues with validation of my model.

To make this more concrete, here is an example that reproduces the problem:

Setup

Consider this model (there are design problems here, but please ignore those):

from pydantic import BaseModel, Field, conint, root_validator
from ipyautoui import AutoUi

class ApertureSettings(BaseModel):
    radius : conint(ge=1) = Field(default=1)
    inner_annulus : conint(ge=1) = Field(default=2)
    outer_annulus : conint(ge=1) = Field(default=3)

    class Config:
        validate_assignment = True
        validate_all = True

    @root_validator(skip_on_failure=True)
    def check_annuli(cls, values):
        if values['inner_annulus'] >= values['outer_annulus']:
            raise ValueError('inner_annulus must be smaller than outer_annulus')
        if values['radius'] >= values['inner_annulus']:
            raise ValueError('radius must be smaller than inner_annulus')
        return values

Next, create a ui and an observer:

ui = AutoUi(ApertureSettings)

counter = 0
def my_observer(change):
    global counter
    counter += 1
    print(f"My observer call {counter}")
    try: 
        ApertureSettings(**ui.value)
    except ValidationError:
        print("    Bad state")
    else: 
        print("    Good state")

ui.observe(my_observer, names="_value")

Finally, set the value of the ui:

ui.value = dict(radius=2, inner_annulus=4, outer_annulus=8)

Expected outcome

My observer will be called once.

Actual outcome

My observer is called 4 times:

# output of ui.value = dict(radius=2, inner_annulus=4, outer_annulus=8)
My observer call 1
    Good state
My observer call 2
    Bad state
My observer call 3
    Bad state
My observer call 4
    Good state

The number of calls is the number of values being changed in the UI widget plus one, I think because of this line.

Partial fix

This can be partly addressed by wrapping the body of ipyautoui.autoipywidget.AutoObject._update_widgets_from_value with a context manager with self.hold_trait_notifications().

When I try that I still end up with my observer being called twice instead of once, but in both calls the value passes a validation check, which fails when I don't make this change.

jgunstone commented 1 year ago

hi @mwcraig - thanks for reporting this -

i think you're getting changes + 1 events because the change in _value is also registering a change: https://github.com/maxfordham/ipyautoui/blob/6a95087a3e08a451fae3900e1ca3c94ba0a7763c/src/ipyautoui/autoipywidget.py#L651-L659

Agree that you should only get 1no change reportied, and agree that with self.hold_trait_notifications() is probs the best way to go. I'll take a look at this when I get a chance.