Visualize whole tree of infections #55
Conversation
|
Pretty cool!!
I'm guessing this is:
I could really use a legend or something! Also, I would have expected to see:
But it looks like this is actually there is one more? This is telling me our default "number of simulated generations" should be 3, not 4? Does "simulated" include the index? I'll break this off as a separate issue (#62) |
poetry.lock
Outdated
There was a problem hiding this comment.
I got an error about matplotlib, see #61
|
Re: legend and what things are: yes we need one, but I figured we should agree on the aesthetics we liked first. E.g., I also have #64 (which is what I thought I had opened this PR on... oops... but you've got the right interpretation for what's been shown here)
which I think is in many ways better, but requires shapes for detection. |
|
To-dos and to-considers from sync disc:
|
works rename use new plot dropped these update deps cleanup #53 use get_person_properties() everywhere clean up plotting add matplotlib (#61) cleanup per @swo #55 bug fix requires test changes triangles precommit let counterfactual show passive detection, remove unused par put axis back where it belongs, optional gen plotting
afmagee42
force-pushed
the
afm-viz-tree
branch
from
5d76a0c to
733c4f4
Compare
|
Okay @swo and @paigemiller, this is ready for a look now, your feedback has been incorporated. Scott, the generations get crowded fast so "please show me the generation" is an Advanced Setting. |
|
Pulling out some images to reference the new style:
|
There was a problem hiding this comment.
Overall I think this is good! Of course there are a thousand things that could be tweaked; but we should leave those aside.
Two things that you could change or leave aside:
- It makes more sense to me to have the "escape" infections be little black ticks, rather than blue ones?
- "Show infection times?" would make more sense to me as a toggle, rather than a button
| @@ -3,5 +3,5 @@ | |||
|
|
|||
| def test_app(): | |||
| # Cf. https://docs.streamlit.io/develop/api-reference/app-testing | |||
| at = AppTest.from_file("ringvax/app.py").run() | |||
| at = AppTest.from_file("ringvax/app.py", default_timeout=10.0).run() | |||
There was a problem hiding this comment.
This imposes some sort of time limit on how long the tests are allowed to run.
The new plots are apparently slower to render, enough so that while they passed on my machine, whatever the actions are run on needed more time. My guess is the actions must be rendering all the plots for all the simulations, because it's not that slow.
Resolves #24 and #15. Solves #53 along the way because the broken plots that led to were driving me crazy.
Code lives in
ringvax.plot, and I'm rather tempted to wrap it all up into a plotting object since that would avoid a lot of unneeded passing of objects around (the simulation, plotting parameters, and the plot itself).Colors and styles can and probably should be argued over. I strove to keep things clean and visible first and foremost, but also to use our color palette and keep things accessible. I think this is mostly achieved, except perhaps marking detections which is a bit ugly. I tried delineating detection types by color, instead of shape, but I ran out of colors that were both easy to see on both latent and exposed backgrounds and which were truly distinctive from each other.
To keep some of the plotting-induced post-processing sane, I added infection attributes: the
idand theinfectees.