Improve plot output (hatching, unit formatting, mean lines, etc.)#171
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #171 +/- ##
==========================================
- Coverage 83.31% 80.00% -3.32%
==========================================
Files 15 15
Lines 737 770 +33
==========================================
+ Hits 614 616 +2
- Misses 123 154 +31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Just starting a review here: first thing to say is that I really like the new plots! |
andreww
left a comment
There was a problem hiding this comment.
Looks good to me. Strikes me that the main plotting function does not contain any type hints but I think fixing that belongs in a different PR (if we want to be able to save a figure in a command line environment that will probably touch the arguments so let's fix that then - I'll add a comment to the relevant issue).
Also, I have everything running locally with python 3.14 so I'll bump the CI later too
| ax.scatter(now_times, now_values, color=now_colour, s=8, alpha=0.3) | ||
| ax.scatter(opt_times, opt_values, color=optimal_colour, s=8, alpha=0.3) | ||
|
|
||
| def tick_formatting(x, pos): |
There was a problem hiding this comment.
Super minor comment but I would probably call this something like "readable_datetime_tick_formatter" (and then curse because I failed to find a reasonably short name)
There was a problem hiding this comment.
Good point, agreed that the title is too generic and doesn't explain it's purpose very well. If you can note that (or link to your comment here) on the new relevant issue, that would be great. Thanks for the prompt review :)
Address points in #153, excluding unit tests for plotting (best set those up once we agree on what the output plot should look like, IMO, to save any need to continuously tweak the desired output images to test against), with some further plotting improvements thrown in for good measure (call me sad but I enjoy making a nice plot 🙂 ).
All up for discussion and happy to edit or revert anything if folks don't like it - but I for one think the time series plot (shown with
--plot) from our POC plot PR is much more readable, accessible and good-looking after these updates.Updates
This PR implements the following:
Before and after
I used the following command near to time of posting to test:
cats -d 100 --loc RG2 --plot('no overlap' examples) andcats -d 600 --loc RG2 --plot(overlap examples). Starting with after, since that's the output from this PR so most relevant...After (result on this/PR branch)
Example with no overlap of windows
Example with overlap of windows
Before (result on
mainbranch)Example with no overlap of windows
Example with overlap of windows