DASDAE / dascore

A python library for distributed fiber optic sensing
Other
80 stars 17 forks source link

refactor wiggle plot #269

Closed d-chambers closed 1 year ago

d-chambers commented 1 year ago

Description

This PR fixes issue #114 and simplifies the wiggle plot code. They look slightly different now, but the docs correctly display the images and the plotting is much more efficient.

image

@aissah, could you please take a look at this sometime in the next week or so and let me know what you think?

Checklist

I have (if applicable):

codecov[bot] commented 1 year ago

Codecov Report

Merging #269 (1c62973) into master (d786f84) will increase coverage by 0.16%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #269      +/-   ##
==========================================
+ Coverage   99.10%   99.27%   +0.16%     
==========================================
  Files          79       79              
  Lines        6487     6476      -11     
==========================================
  Hits         6429     6429              
+ Misses         58       47      -11     
Flag Coverage Δ
unittests 99.27% <100.00%> (+0.16%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
dascore/viz/wiggle.py 100.00% <100.00%> (+20.00%) :arrow_up:

... and 1 file with indirect coverage changes

d-chambers commented 1 year ago

Todo:

aissah commented 1 year ago

I fixed the density of the labels on the y-axis and made it a maximum of 30. I added the option for shading the peaks and set the alpha for that to 0.6 because it seemed the most visually appealing to me, maybe you can take a look at that. @d-chambers

d-chambers commented 1 year ago

Hmmm, still doesn't look quite right for a small number of traces:

image

In this case I would expect the distance labels to line up with the center of the traces. I think I have an idea how to fix it though, I will give it a go.

aissah commented 1 year ago

Did the last commit fix it?

d-chambers commented 1 year ago

Did the last commit fix it?

No, I just added a test to make sure the shading part of the code gets run. I am still working on it.

d-chambers commented 1 year ago

Ok, I played around with plotting several kinds of patches and made two small adjustments:

  1. I added alpha as a parameter to wiggle. I hate to add more parameters but I found this was one I needed to adjust depending on the number of channels, how much they were touching, etc.
  2. I changed the max number of y axis labels from 30 to 10, which seemed more reasonable for some smaller sized plots, but the larger ones still looked fine. These can also be manipulated manually with the returned axis.

I couldn't quite get the plot above to look good because the date axis formatting is different from a non-date axis. It is probably an edge case that doesn't merit a lot of time though.

Overall, I think there are still some improvements we can make in the future but to me it looks good for now.

d-chambers commented 1 year ago

I am going to merge this for now, we can make any changes in a future PR.