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
set default value from signature defaults in interact #5136
Conversation
If available, use the default value from the signature for the initial condition, when using range/choice abbreviations. Not affected: - single-value abbreviations (`@interact(a=5)` sets `a=5`) - explicit Widgets
This works perfect for me, thank you! |
implementation looks sound, and the tests are passing. I'll check again at my desk tomorrow and then merge. |
try: | ||
widget.value = default | ||
except TraitError: | ||
# warn? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes... I think it would be good to have an advice here...
LGTM 👍 |
We should also handle the case where the abbrev is a dict and add a test for that. |
both valid and invalid
dict case is tested, and a bug revealed in selection widgets is fixed as a result. |
before raising otherwise, invalid values were still accepted
# Now build the widgets from the abbreviations. | ||
kwargs_widgets.extend(_widgets_from_abbreviations(new_kwargs)) | ||
kwargs_widgets.extend(_widgets_from_abbreviations(sorted(kwargs.items(), key = lambda x: x[0]))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove the sorting? Is the idea to follow the order of arguments defined in the function itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not removing the sorting, it's removing the line altogether. This line did nothing because kwargs is always empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, great.
Looks good merging. |
set default value from signature defaults in interact
set default value from signature defaults in interact
If available, use the default value from the signature for the initial condition, when using range/choice abbreviations.
Not affected:
@interact(a=5)
setsa=5
)as proposed in #5134