Handle changes in kwarg defaults in Axes.scatter (take 2)

[duncanmmacleod]

This PR revives #328.

[codecov]

Codecov Report

Merging #333 (c924008) into master (043bdec) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #333   +/-   ##
=======================================
  Coverage   49.52%   49.52%           
=======================================
  Files          57       57           
  Lines        8420     8420           
=======================================
  Hits         4170     4170           
  Misses       4250     4250           

Flag	Coverage Δ
Linux	49.52% <ø> (?)
macOS	49.52% <ø> (ø)
python3.10	49.52% <ø> (ø)
python3.7	48.68% <ø> (?)
python3.8	49.52% <ø> (?)
python3.9	49.52% <ø> (?)

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

Impacted Files	Coverage Δ
gwsumm/plot/triggers/__main__.py	98.10% <ø> (ø)
gwsumm/plot/triggers/core.py	17.77% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[eagoetz]

@duncanmmacleod Thanks for the revised PR to make this work. A couple items:

• The changes look straightforward fixes.
• Do you have an example to prove this works?
• Can you re-run the CI tests? It looks like a lot of them have failures

Thanks again!

[duncanmmacleod]

@duncanmmacleod Thanks for the revised PR to make this work. A couple items:

• The changes look straightforward fixes.
• Do you have an example to prove this works?

No. I will endeavour to remember how to run my own code to generate one...

• Can you re-run the CI tests? It looks like a lot of them have failures

Done, green CI across the board!

Thanks again!

[eagoetz]

@duncanmmacleod A gentle nudge on this. Waiting for an example to prove this works, if it is possible to generate. Thanks!

[eagoetz]

Sorry for the ping @duncanmmacleod, I know you were on holiday. Just checking back with you on the status of this and wanted to make sure it didn't fall off your radar. Thanks

[robertbruntz]

@duncanmmacleod A second to Evan's nudge.

[duncanmmacleod]

Gents, I've rebased this again, updating the GWpy requirement to 3.0.0, will try and remember how to generate an example.

[eagoetz]

Gents, I've rebased this again, updating the GWpy requirement to 3.0.0, will try and remember how to generate an example.

Hi @duncanmmacleod any progress on this front? Thank you!

[eagoetz]

@duncanmmacleod I want to try and get the new release of gwsumm out this week, maybe tomorrow. Do you think that you will be able to address this PR this week? If so, that would be really wonderful. Thanks in advance!

[eagoetz]

@duncanmmacleod I'm not sure this is enough to fix the issue. I've copied the changes in this PR and made a summary page for the omicron triggers recently from H1: https://ldas-jobs.ligo-wa.caltech.edu/~evan.goetz/summary/day/20221122/lock/glitches/

In the upper left plot, the triggers with higher SNR are still being obscured by the lower SNR triggers. 😞 Somehow the sorting is still not working properly

Edit: I stand corrected! It actually does work if I add sortbycolor = True to the summary page configuration files. Now that I did that it does work as intended.

[eagoetz]

LGTM

[robertbruntz]

Note that in order to plot higher SNR points on top of lower SNR points in VET plots, the updated code requires the config file to include "glitchgram-sortbycolor = True" in the "[DEFAULT]" section (e.g., here), as noted here. (I think regular Summary Pages just use sortbycolor = True.)