vizro icon indicating copy to clipboard operation
vizro copied to clipboard

[Tidy] Convert actions to classes

Open petar-qb opened this issue 11 months ago • 0 comments

Description

For those who will review this PR:

Here is a small recommendation on how to familiarise yourself with the PR changes in the fastest way and how to review the PR. It is only a recommendation and feel free to skip some of the suggested steps if you are already familiar with them or if you are not interested in them.

  1. Don't rush to review the changes right away.
  2. Read the entire PR description.
  3. Watch the Team Learning session.
  4. Run all examples in vizro-core/examples/_dev/ folder. They are already sorted alphabetically in the way they should run. For each example: read file docstrings, explore application configuration, and run and play with application UI.
  5. Investigate CapturedActionCallable → useful Gist that @antonymilne made: https://gist.github.com/antonymilne/174c4c49ace2dae1f1d7674183b8d140 Link to the PS session -> https://mckinsey.box.com/s/fg1sdrpw6emprihhtg19ww4lft2ljma8
  6. Open and understand one of the action implementations (e.g. export_data_action)
  7. Search for TODO-AV2 and read everything. (especially since some "TODO-AV2" refer to multiple similar pieces of code, but are only written in one place)
  8. Start with discussing open questions ("TODO-AV2-OQ")
  9. Feel completely free to contribute:
    • Implementing some better architecture solutions,
    • Open any new kind of question.
    • Solving TODOs,
    • Cleaning the code,
    • Applying linting stuff,
    • Fixing/adding tests
    • Literally anything else
  10. Start with the standard code reviewing.

This PR is in the MVP phase because:

  • There are still some open questions that should be discussed.
  • The code is not completely tidied up.
  • tests, docs and linting is not completed.

The most important changes:

  • Implementation of class based action using CapturedActionCallable.
  • Actions validation and eager input arguments calculation.
  • Make filter_action and parameter_action public,
  • Change _on_page_load with a public update_figures action.
  • Expose Page.actions so it can be overwritten.
  • model_manager._get_model_page_id method improved

Bugfixes:

  • Enable any Vizro model to work with actions in the control panel (https://github.com/mckinsey/vizro/issues/376) (model_manager._get_page_actions_chains improved)
  • export_data is not able anymore to export data from other pages anymore (the bug is not spotted earlier and is fixed in this PR),

TODO-AV2 legend:

  • TODO-AV2 - I will solve these as the part of this PR,
  • TODO-AV2-OQ- Open Questions,
  • TODO-AV2-TICKET-CREATED - These will be solved as a separate PR, and the ticket for these is already created,
  • TODO-AV2-TICKET-NEW - These will be solved as a separate PR, and the following tickets have to be created,
  • *- tagged TODOs (contain the characters -*:) are candidate for solving by anyone from the team.

PR TODOs:

  • the final tidying up (getting rid of code duplications and some TODOs)
  • tests
  • docs

Notice

  • [x] 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.

petar-qb avatar Mar 12 '24 13:03 petar-qb