-
Notifications
You must be signed in to change notification settings - Fork 107
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
Enable _clear_state
and ensure test independence
#120
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks you two for fixing @maxschulz-COL @ned2! 🚀
@maxschulz-COL - could you add @ned2 to our list of contributors in our authors.md file to give him credits for the proposed solution?
vizro-core/changelog.d/20231019_130629_maximilian_schulz_implement_improved_clearing.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for cleaning up! 🧹 🔥
…/vizro into bug/implement_improved_clearing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the fix @maxschulz-COL and @ned9!
I just have a couple of small comments and one concern about the test fixture but otherwise looks great 💯
In the near future I think we might want to move some of this resetting behaviour so it always runs automatically in our Vizro.__init__
or similar, and also make the rest that's left in Vizro._reset
into public API. This can be handled as part of #111 because it's all related.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good!
Co-authored-by: Antony Milne <49395058+antonymilne@users.noreply.github.com> Signed-off-by: Maximilian Schulz <83698606+maxschulz-COL@users.noreply.github.com>
Description
_clear_state
function as suggested by @ned2tech_logos.png
Screenshot
Checklist
Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1))
(if applicable)Types of changes
Notice
I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":