Skip to content

update data and value together - fixes #634#637

Merged
AnnMarieW merged 10 commits intosnehilvj:masterfrom
AnnMarieW:fix-634-mult-select
Aug 24, 2025
Merged

update data and value together - fixes #634#637
AnnMarieW merged 10 commits intosnehilvj:masterfrom
AnnMarieW:fix-634-mult-select

Conversation

@AnnMarieW
Copy link
Copy Markdown
Collaborator

@AnnMarieW AnnMarieW commented Aug 23, 2025

closes #634

  • fixes 'MultiSelectandSelectso that changes to thedataandvalue` are batched so they only trigger a single callback.

@AnnMarieW AnnMarieW requested a review from alexcjohnson August 23, 2025 18:51
Comment thread CHANGELOG.md Outdated
Copy link
Copy Markdown
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 nice tests!

@AnnMarieW
Copy link
Copy Markdown
Collaborator Author

AnnMarieW commented Aug 23, 2025

Hi @alexcjohnson
Thanks for the review! Just as you approved it, I thought of a more efficient way to do this. I made it so it updates the data and value together only when the data changes. (Rather than when either the data or value changes) The data doesn't get updated in a callback frequently. Do you see any issues with this?

AnnMarieW and others added 2 commits August 23, 2025 16:18
@alexcjohnson
Copy link
Copy Markdown
Collaborator

Ah yes, that's better, good thinking!

Makes me realize though, it might be overkill but in the tests in principle just looking at ctx.triggered leaves open the possibility that the callback was executed more than once and this is just the last time, so to be precise (given that the bug is about unnecessary callbacks) you really want confirmation that there were no other calls. You could do this either by counting the callback invocations (with multiprocessing.Value) or by accumulating ctx.triggered (take the previous dmc-triggered.children as State and add the new one to it).

@AnnMarieW
Copy link
Copy Markdown
Collaborator Author

Good point!
Accumulating ctx.triggered makes sense to me. I added that to both tests.

@AnnMarieW AnnMarieW merged commit 643c896 into snehilvj:master Aug 24, 2025
1 check passed
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.

[bug] dmc.MultiSelect duplicate callback triggers from one action

2 participants