gwpy / gwsumm

Gravitational-wave interferometer summary information system
GNU General Public License v3.0
12 stars 23 forks source link

Add default `rcParams` #386

Closed iaraota closed 7 months ago

iaraota commented 10 months ago

This pull request introduces default rcParams, ensuring that plots have the desired appearance without the necessity of activating the ligo-summary-3.10 Conda environment.

These parameters were obtained by comparing the matplotlib.rcParams differences between igwn and ligo-summary-3.10 Conda environments.

iaraota commented 7 months ago

@eagoetz I tested the following:

import matplotlib.pyplot as plt
import numpy as np
import matplotlib as mpl

plt.rcParams["figure.figsize"] = (5,10)
data = np.random.randn(50)
plt.plot(data)
plt.savefig('before_gwsumm.png')
import gwsumm
plt.close('all')
plt.plot(data)
plt.savefig('before_after.png')

The saved figures remain unchanged, that is, the "figure.figsize" stays at (5,10) rather than the GWSumm default of (12, 6).

eagoetz commented 7 months ago

@iaraota Thanks for trying this out! Have you tried from gwsumm import plot? I want to confirm what that does with this PR. Thanks!

Also, I'm not sure what's going on with the CI failures, but it is likely related to this change.

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (a50504f) 50.17% compared to head (09601f5) 49.96%. Report is 4 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #386 +/- ## ========================================== - Coverage 50.17% 49.96% -0.21% ========================================== Files 60 60 Lines 8685 8729 +44 ========================================== + Hits 4357 4361 +4 - Misses 4328 4368 +40 ``` | [Flag](https://app.codecov.io/gh/gwpy/gwsumm/pull/386/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gwpy) | Coverage Δ | | |---|---|---| | [Linux](https://app.codecov.io/gh/gwpy/gwsumm/pull/386/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gwpy) | `49.96% <100.00%> (-0.21%)` | :arrow_down: | | [python3.10](https://app.codecov.io/gh/gwpy/gwsumm/pull/386/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gwpy) | `49.96% <100.00%> (-0.21%)` | :arrow_down: | | [python3.11](https://app.codecov.io/gh/gwpy/gwsumm/pull/386/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gwpy) | `49.96% <100.00%> (-0.21%)` | :arrow_down: | | [python3.9](https://app.codecov.io/gh/gwpy/gwsumm/pull/386/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gwpy) | `49.96% <100.00%> (-0.21%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gwpy#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

iaraota commented 7 months ago

@eagoetz thank you for your suggestion. I tested the following, and the plots remain the same:

import matplotlib.pyplot as plt
import numpy as np
import matplotlib as mpl

plt.rcParams["figure.figsize"] = (5,10)
data = np.random.randn(50)
plt.plot(data)
plt.savefig('before_gwsumm.png')
from gwsumm import plot
plot.DataPlot(['L1:DMT-SNSL_EFFECTIVE_RANGE_MPC'], 1388102418, 1388188818)
plt.close('all')
plt.plot(data)
plt.savefig('before_after.png')

I fixed the test. It was checking whether cp.set('plot', 'figure.figsize', '2, 1') works. However, since rcParams used to be empty, is was asserting the rcParams as a dictionary. Now I am testing only the change in figure.figsize. By the way, this test also shows that rcParams is changeable, as you were concerned it could not be.