-
Notifications
You must be signed in to change notification settings - Fork 1
new expandable_list ui element #600
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
Open
AurelienJaquier
wants to merge
17
commits into
main
Choose a base branch
from
new_ui_element
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
182106b
new expandable_list ui element
AurelienJaquier 705a7ab
complex variable holder implementation
AurelienJaquier 3c65fb6
change ui element name to voltage_duration_ui_element
AurelienJaquier 97a0df9
move new block into block_subunit
AurelienJaquier c829600
Merge branch 'main' of https://github.com/openbraininstitute/obi-one …
AurelienJaquier 2c80337
tmp commit
AurelienJaquier 6f85ad2
fix scan parameter with ComplexVariableHolder
AurelienJaquier 462e205
update docs
AurelienJaquier 6c555e7
Merge branch 'main' of https://github.com/openbraininstitute/obi-one …
AurelienJaquier 7e7784a
remove old file
AurelienJaquier 1c09eef
fix test
AurelienJaquier 582f048
improve test
AurelienJaquier e3a420a
Merge branch 'main' of https://github.com/openbraininstitute/obi-one …
AurelienJaquier 2be187c
Merge branch 'main' of https://github.com/openbraininstitute/obi-one …
AurelienJaquier 328f21b
answer comments
AurelienJaquier 808542b
Merge branch 'main' of https://github.com/openbraininstitute/obi-one …
AurelienJaquier 59d6e91
lint fix
AurelienJaquier File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Binary file added
BIN
+30.4 KB
.../components/voltage_duration_ui_element/designs/voltage_duration_ui_element.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
39 changes: 39 additions & 0 deletions
39
...components/voltage_duration_ui_element/reference_schemas/voltage_duration_ui_element.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| { | ||
| "duration_voltage_combinations": { | ||
| "items": { | ||
| "duration": { | ||
| "oneOf": [ | ||
| { | ||
| "type": "float" | ||
| }, | ||
| { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "float" | ||
| } | ||
| } | ||
| ], | ||
| "ui_element": "float_parameter_sweep" | ||
| }, | ||
| "voltage": { | ||
| "oneOf": [ | ||
| { | ||
| "type": "float" | ||
| }, | ||
| { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "float" | ||
| } | ||
| } | ||
| ], | ||
| "ui_element": "float_parameter_sweep" | ||
| } | ||
| }, | ||
| "type": "array", | ||
| "minLength": 1 | ||
| }, | ||
| "title": "Voltage Levels and Durations", | ||
| "description": "A list of duration and voltage combinations for the SEClamp stimulus.", | ||
| "ui_element": "voltage_duration_ui_element" | ||
| } |
55 changes: 55 additions & 0 deletions
55
...tion-spec/components/voltage_duration_ui_element/voltage_duration_ui_element.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| ## Voltage duration UI element | ||
|
|
||
| ui_element: `voltage_duration_ui_element` | ||
|
|
||
| Reference schema [voltage_duration_ui_element](reference_schemas/voltage_duration_ui_element.json) | ||
|
|
||
|
|
||
| ### UI design | ||
|
|
||
| <img src="designs/voltage_duration_ui_element.png" width="300" /> | ||
|
|
||
| The user can add new DurationVoltageCombination by clicking on a button. | ||
|
|
||
| They can delete any DurationVoltageCombination, unless there is only one left, in which case it cannot be deleted. | ||
|
|
||
|
|
||
| ### Example Pydantic implementation | ||
|
|
||
| ```py | ||
|
|
||
| class DurationVoltageCombination(ComplexVariableHolder): | ||
| """Class for storing pairs of duration and voltage combinations for stimulation protocols.""" | ||
|
|
||
| voltage: float | list[float] = Field( | ||
| title="Voltage for each level", | ||
| description="The voltage for each level, given in millivolts (mV).", | ||
| json_schema_extra={ | ||
| "ui_element": "float_parameter_sweep", | ||
| "unit": "mV", | ||
| }, | ||
| ) | ||
|
|
||
| duration: NonNegativeFloat | list[NonNegativeFloat] = Field( | ||
| title="Duration for each level", | ||
| description="The duration for each level, given in milliseconds (ms).", | ||
| json_schema_extra={ | ||
| "ui_element": "float_parameter_sweep", | ||
| "unit": "ms", | ||
| }, | ||
| ) | ||
|
|
||
|
|
||
| class Block: | ||
|
|
||
| duration_voltage_combinations: list[DurationVoltageCombination] = Field( | ||
g-bar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| title="Voltage Levels and Durations", | ||
| description="A list of duration and voltage combinations for the SEClamp stimulus. " | ||
| "The first duration starts at time 0, " | ||
| "and each subsequent duration starts when the previous one ends.", | ||
| json_schema_extra={ | ||
| "ui_element": "voltage_duration_ui_element", | ||
| }, | ||
| ) | ||
|
|
||
| ``` | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Empty file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| from pydantic import Field, NonNegativeFloat, PrivateAttr | ||
|
|
||
| from obi_one.core.base import OBIBaseModel | ||
| from obi_one.core.param import MultiValueScanParam | ||
| from obi_one.core.schema import SchemaKey, UIElement | ||
| from obi_one.core.units import Units | ||
|
|
||
|
|
||
| class ComplexVariableHolder(OBIBaseModel, extra="forbid"): | ||
| """Has structure List[Tuple[AnyOf(int, float, list[float], list[int])]].""" | ||
|
|
||
| _multiple_value_parameters: list[MultiValueScanParam] = PrivateAttr(default=[]) | ||
|
|
||
| # checks this for parameter scan | ||
| def multiple_value_parameters(self, base_location_list: list[str]) -> list[MultiValueScanParam]: | ||
| self._multiple_value_parameters = [] | ||
|
|
||
| for key, value in self.__dict__.items(): | ||
| multi_values = [] | ||
| if isinstance(value, list): | ||
| multi_values.append( | ||
| { | ||
| "value": value, | ||
| "location_list": [*base_location_list, key], | ||
| } | ||
| ) | ||
|
|
||
| for multi_value in multi_values: | ||
| self._multiple_value_parameters.append( | ||
| MultiValueScanParam( | ||
| location_list=multi_value["location_list"], values=multi_value["value"] | ||
| ) | ||
| ) | ||
|
|
||
| return self._multiple_value_parameters | ||
|
|
||
|
|
||
| class DurationVoltageCombination(ComplexVariableHolder): | ||
| """Class for storing pairs of duration and voltage combinations for stimulation protocols.""" | ||
|
|
||
| voltage: float | list[float] = Field( | ||
| title="Voltage for each level", | ||
| description="The voltage for each level, given in millivolts (mV).", | ||
| json_schema_extra={ | ||
| SchemaKey.UI_ELEMENT: UIElement.FLOAT_PARAMETER_SWEEP, | ||
| SchemaKey.UNITS: Units.MILLIVOLTS, | ||
| }, | ||
| ) | ||
|
|
||
| duration: NonNegativeFloat | list[NonNegativeFloat] = Field( | ||
| title="Duration for each level", | ||
| description="The duration for each level, given in milliseconds (ms).", | ||
| json_schema_extra={ | ||
| SchemaKey.UI_ELEMENT: UIElement.FLOAT_PARAMETER_SWEEP, | ||
| SchemaKey.UNITS: Units.MILLISECONDS, | ||
| }, | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
I don't think we need to nest
ui_element(at least for now), the FE can simply hardcode the ParameterSweep components for the DurationVoltageCombination, in fact this isn't in the reference schema, so it's not even visible to the FE. (Or if you want it make sure it's in the reference schema).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.
added
"ui_element": "float_parameter_sweep",to the reference schema, in both voltage and duration so that it is visible to the FEUh oh!
There was an error while loading. Please reload this page.
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.
There is no use for this nested ui_element, since the 'voltage' and 'duration' fields are hardcoded in the schema there is no point to "dynamically" render the inner elements since they never change for the voltage duration ui_element, this would only useful with a "generic" ui_element where the inner fields can vary.
I'm not sure how even making this generic would work, since the pydantic models are static so there is no way to define the field names dynamically.
So it should be removed from the schema, I will just ignore it for now, and hardcode ParameterSweep component for both of those fields on the FE.