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

Picking input prop(s) #6

Closed
alexcjohnson opened this issue Jan 22, 2021 · 7 comments
Closed

Picking input prop(s) #6

alexcjohnson opened this issue Jan 22, 2021 · 7 comments

Comments

@alexcjohnson
Copy link
Collaborator

For changing the prop of an input from the default value, right now we have eg:

dcc.DatePickerSingle().props["date"]
dcc.DatePickerRange().props[("start_date", "end_date")]

The great thing about this syntax vs dcc.DatePickerSingle().date is you can easily pick multiple values and pack them into either a list or a dict for your function - and we should allow you to do either of those with the regular id/prop callback definitions too (though list packing would probably have to be a breaking change due to how we unlisted outputs, inputs, and state in dash 1.x)

I don't get why it's formatted to look like a dict though - to me it's more like a method modifying the component's "value" to be some prop(s) other than value.

Also: what if you want one prop as input and another as state? For example a dcc.Store with data as state, modified_timestamp as input? And per #5, what if you want modified_timestamp to be a Trigger, or hidden? We could imagine making tuples:
dcc.Store().props(("modified_timestamp", "state"), "data")
or take advantage of the fact that props can't have dots in them:
dcc.Store().props("modified_timestamp.hidden", "data")

@jonmmease
Copy link
Contributor

(though list packing would probably have to be a breaking change due to how we unlisted outputs, inputs, and state in dash 1.x)

There is no list packing, only tuples. So I don't think there's any incompatibility.

Also: what if you want one prop as input and another as state?

Yeah, this is a compelling argument for another approach. With the current approach, I like the symmetry of the packing structure between the property specification and the function input value. So I think I'd prefer encoding the "stateness" in the string somehow.

dcc.Store().props[("modified_timestamp.state", "data")] or something.

I used __getitem__ lookup because I thought of the operation as accessing properties in the component. It returns a different type that the component: ComponentProps.

But I don't feel that strongly about parens vs brackets. I like brackets a little more since I think it looks better when using tuple packing (no consecutive parens).

@alexcjohnson
Copy link
Collaborator Author

There is no list packing, only tuples. So I don't think there's any incompatibility.

But right now in @app.callback we unpack either lists or tuples, as part of plotly/dash#1180, specifically this line:
flat_args += arg if isinstance(arg, (list, tuple)) else [arg]

I like brackets a little more since I think it looks better when using tuple packing (no consecutive parens).

If we allow *args, **kwargs then .props() would generate a single value as the callback arg when supplied a single unnamed param:
dcc.Store().props("data") => data

It would generate a tuple when supplied multiple unnamed params:
dcc.Store().props("modified_timestamp.state", "data") => (ts, data)

And it would generate a dict when supplied even a single named param:
dcc.Store().props(ts="modified_timestamp.state", d="data") => {"ts": ts, "d": data}

I suppose mixing named and unnamed args would be an error.

@jonmmease
Copy link
Contributor

If we allow *args, **kwargs ...

Oh, I see. I thought you were thinking that something about the *args or **kwargs here had to do with flagging a prop as State. But args to tuple, kwargs to dict, with not mixing, makes sense.

But right now in @app.callback we unpack either lists or tuples, as part of plotly/dash#1180

doh. I guess that would be a 2.0 backward incompatibility. Really too bad we didn't come up with this design before inputs/output lists became optional. This makes things a lot messier.

Return values of tuples are still different from lists right? e.g. you can't return a tuple to satisfy multiple outputs can you?

@alexcjohnson
Copy link
Collaborator Author

e.g. you can't return a tuple to satisfy multiple outputs can you?

I believe you can return either a tuple or list - for the most part we have not made a distinction between these. Isn't that what you get when you do return a, b? I know that's valid for multi-output callbacks.

@jonmmease
Copy link
Contributor

Ok, There's more incompatibility here than I thought then.

@jonmmease
Copy link
Contributor

I do have one issue with this props function syntax

dcc.Store().props("modified_timestamp.state", "data") => (ts, data)

I'm assuming that the scalar string case would produce a scalar prop value:

dcc.Store().props("data") => data

But then there's no way to represent length-1 tuples. Granted, directly specifying length-1 tuples is an unusual thing to do, but when writing generic code that generates these inputs/outputs from plugins, I think it would be more common.

So I think I still prefer the getitem syntax where

dcc.Store().props["data"] => data
dcc.Store().props[("data",)] => (data,)

@jonmmease
Copy link
Contributor

no longer applicable to current design

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

No branches or pull requests

2 participants