Add docstring for jupyterviz make_user_input that documents supported inputs #1784
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1784 +/- ##
==========================================
+ Coverage 79.58% 79.88% +0.29%
==========================================
Files 15 15
Lines 877 880 +3
Branches 188 188
==========================================
+ Hits 698 703 +5
+ Misses 158 154 -4
- Partials 21 23 +2
☔ View full report in Codecov by Sentry. |
mesa/experimental/jupyter_viz.py
Outdated
| @@ -133,28 +133,37 @@ def function(viz): | |||
| return function | |||
|
|
|||
|
|
|||
| def make_user_input(user_input, k, v): | |||
| if v["type"] == "SliderInt": | |||
| def make_user_input(user_input, options): | |||
There was a problem hiding this comment.
I initially included the name ("k") because I figured it might be used in the future. Looking at the code again, I think the name can be used as a fallback label instead of the current generic "label".
There was a problem hiding this comment.
Ah, ok, that makes sense; I'll revise.
Should the method throw an exception if the type is not supported? If so, does ValueError seem appropriate to you?
There was a problem hiding this comment.
@rht updated with name fallback logic, exception for unsupported type, and preliminary unit test for this method
|
This PR LGTM, but I'm merging #1786 first because that PR is a huge refactor, will cause a merge conflict, and it is easier to redo this PR instead of that one. Sorry for this. |
|
There is indeed a merge conflict. |
|
@rht thanks for letting me know. I'll see what I can figure out; looking forward to seeing the updates. |
Resolve merge conflict with recent jupyterviz refactor
rlskoeser
force-pushed
the
document-jupyterviz-userinput
branch
from
9cc867e to
1bd1f73
Compare
|
Merged, thank you @rlskoeser ! |
make_user_inputmethod that documents the supported solara inputsmake_user_input- drop unused parameter, use more readable variable namessome related conversation on #1772 and follow up work to #1777
I haven't added any tests yet since there is no existing testing infrastructure for the jupyterviz stuff, but I did test locally with a simulation that uses a slider int and a switch parameter and the changes work