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

feat: draft autowidget #54

Closed
wants to merge 4 commits into from
Closed

feat: draft autowidget #54

wants to merge 4 commits into from

Conversation

jokasimr
Copy link
Contributor

@jokasimr jokasimr commented Aug 1, 2024

Related to #28

Experiment with AutoWidget helper to make writing widgets for parameters a bit easier.

The AutoWidget class creates a composite widget from a constructor and a range of argument widgets.
The resulting widget is a simple form that produces a value of the constructor type.

@jokasimr jokasimr marked this pull request as draft August 1, 2024 11:03
@jokasimr jokasimr requested review from MridulS and YooSunYoung August 1, 2024 11:04
@jokasimr
Copy link
Contributor Author

jokasimr commented Aug 1, 2024

Not sure how far we can get with this approach or a similar one, but to avoid having to create lots of very similar widgets for different workflow parameters it would be nice to automate what we can

@jokasimr
Copy link
Contributor Author

jokasimr commented Aug 1, 2024

Screenshot from 2024-08-01 13-13-27

@SimonHeybrock
Copy link
Member

but to avoid having to create lots of very similar widgets for different workflow parameters

Can you clarify your concern? I didn't think there would be too many different widgets.

@jokasimr
Copy link
Contributor Author

jokasimr commented Aug 2, 2024

There's some bookkeeping involved in making a widget that creates a composite value from a number of underlying basic widgets. It's good if we can avoid repeating it unnecessarily.

In particular, the AutoWidget let's you observe the composite value, and it's updated when any of the underlying values change. That didn't happen in the old widget implementations. That makes it easier to compose widgets together.

There are other ways to achieve that, not sure if this is the best, but I found it to work quite well so far.

@SimonHeybrock SimonHeybrock changed the base branch from main to param-class August 2, 2024 06:03
@jokasimr
Copy link
Contributor Author

jokasimr commented Aug 2, 2024

I didn't think there would be too many different widgets.

Not necessarily too many, I agree. I thought there would be more than there seems to be. It might be easier to just keep the widget implementations separate

@SimonHeybrock
Copy link
Member

If I understood correctly during the standup we will not pursue this for now, may be revisited later?

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

Successfully merging this pull request may close these issues.

2 participants