[go: up one dir, main page]

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] Enable dynamic filter #879

Merged
merged 77 commits into from
Dec 2, 2024
Merged

[Feat] Enable dynamic filter #879

merged 77 commits into from
Dec 2, 2024

Conversation

petar-qb
Copy link
Contributor
@petar-qb petar-qb commented Nov 15, 2024

Description

This PR introduces dynamic filters for the following selectors (so, all Vizro selectors except vm.DatePicker):
vm.Dropdown, vm.RadioItems, vm.Checklist, vm.Slider and vm.RangeSlider

You can test the feature by altering values in the scratch_dev/data.yaml (in the way that's described in the comment) and running the scratch_dev/app.py.

TODOs:

This PR TODOs:

This or next PR TODOs (we should decide it):

  • - Support dynamic for the vm.DatePicker. (Maybe it's better to wait for the dash persistence bugfix as persistence doesn't work for the vm.DatePicker even on the main branch)

Next PRs:

  • - Enhance the default selector.value handling for new users. There are a few cases marked with 🟠 in this Issue -> https://github.com/McK-Internal/vizro-internal/issues/1356. There are two inconsistencies listed at the bottom of the Issue description. This TODO points to the second inconsistency.
  • - Introduce "Universal Vizro placeholder component" -> https://github.com/McK-Internal/vizro-internal/issues/1307
  • - Fix the multi=False vm.Dropdown dynamic selector bug when the value is cleared. This will be solved when Vizro universal placeholder component become introduced.
  • - Enable dynamic filter to work with the empty data_frame.
  • - Propagate data_frame Parameter default values from the model_manager into the DM._multi_load() that's called from the vm.Filter.pre_build(). -> PoC can be found in the comment.
  • - Implement "Select ALL" for the multi=True categorical selectors.
  • - Implement "Select the entire range" for the multi=True numerical selectors.
  • - Enhance filter.build UI for the returned object. -> [Feat] Enable dynamic filter #879 (comment), [Feat] Enable dynamic filter #879 (comment)

References:

Open questions:

  1. How to enable dynamic Parameters? Should we allow, for example, the "options" property of a categorical parameter selector to be a function that dynamically calculates and returns new options?
  2. How to enable a dynamic filter to be targeted by any predefined action? For example the parameter_action target. Or like this: update_figures(targets=[”filter_1_id”])
  3. What exactly is a new way of dash persistence handling we expect (this fix will be implemented by us)?
  4. If we introduce the data_frame property for the vm.Filter component, and if the data_frame Parameters change the targeting form to data_manager_key.function_argument, does is mean that our dynamic Filters could be handled in a same way as any other dynamic figure components?

Notice

  • I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

    • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
    • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
    • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.
    • I have not referenced individuals, products or companies in any commits, directly or indirectly.
    • I have not added data or restricted code in any commits, directly or indirectly.

antonymilne and others added 29 commits November 4, 2024 10:52
# Conflicts:
#	vizro-core/src/vizro/models/_controls/filter.py
…_manager to the Filter.pre_build data_manager _multi_load
@github-actions github-actions bot added the Vizro-AI 🤖 Issue/PR that addresses Vizro-AI package label Nov 15, 2024
Copy link
Contributor
@stichbury stichbury left a comment

Choose a reason for hiding this comment

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

LGTM! A few minor tweaks but happy to approve ahead of any final changes 🥇 🏆

Copy link
Contributor
@maxschulz-COL maxschulz-COL left a comment

Choose a reason for hiding this comment

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

🚀 ⭐ ❤️ Really cool stuff @petar-qb @antonymilne

I have now gone through the code in detail, tried to understand alll code flow with Debugger, and I think this is great.

Obviously it was hard to grasp all the last details without having been in the meetings, but from what I can understand this is really sound!!

I have not yet looked at the tests, can do later today, please let me know if that is required. I am definitely fine with this already. Well done again!

vizro-core/src/vizro/models/_page.py Show resolved Hide resolved
vizro-core/src/vizro/models/_page.py Show resolved Hide resolved
vizro-core/docs/pages/user-guides/data.md Show resolved Hide resolved
vizro-core/src/vizro/_vizro.py Show resolved Hide resolved
vizro-core/src/vizro/models/_components/form/dropdown.py Outdated Show resolved Hide resolved
petar-qb and others added 7 commits November 29, 2024 11:59
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
antonymilne and others added 4 commits November 29, 2024 11:34
Copy link
Contributor
@antonymilne antonymilne left a comment

Choose a reason for hiding this comment

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

Final seal of approval and huge congratulations and applause from me 🎉 :shipit:

@petar-qb
Copy link
Contributor Author

@maxschulz-COL
First of all, thank you very much for reviewing this PR. I guess it wasn't the easiest job since you weren't at the meetings where we discussed the limitations and solutions. 🥇 🙏

I have not yet looked at the tests, can do later today, please let me know if that is required. I am definitely fine with this already. Well done again!

I don't think it's mandatory for you to take a look at the tests. You can quickly skim through them if you want, but it's really optional.

@petar-qb petar-qb merged commit dfafd88 into main Dec 2, 2024
44 checks passed
@petar-qb petar-qb deleted the feat/dynamic-filter branch December 2, 2024 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Vizro-AI 🤖 Issue/PR that addresses Vizro-AI package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants