- Notifications
You must be signed in to change notification settings - Fork22
Test to guard against styling side-effects.#160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
my eyes are bleeding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
How about a normal distribution? They're more aesthetically pleasing.
(Also perhaps a more intelligent test since I'm using theHistogramWidget.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
my eyes were bleeding at the use ofjet mainly, so the histogram is 👍 much better
Uh oh!
There was an error while loading.Please reload this page.
samcunliffe commentedJun 12, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
If you like this, I'll force-push a squash to save the repo size.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nice! Please do squash this into one commit, and feel free to self-merge when that's done
Use plt.subplots for safety.And no longer need copy.deepcopy.Save eyes and make a lovely histogram.
I think the auto-merge is smart enough to disregard your approval because the commit hash changed (?) |
Developed along the way to battle-hardening#156.
Here is a test to guard against re-introducing#64.
Relates to
Testing
Can be tested by dumping a
matplotlib.style.use("Solarize_Light2")in the middle of the test function... causing it to fail as expected with:Notes to the reviewer
Doesn't actuallyhave to go onto
main, but... it can do. And doesn't hurt? Especially if we abandon#2. In any case, I will cherry-pick it over onto#156.I couldn't think of a smart way to freeze the style and compare pre- vs post- style change... but a dumb
mark.mpl_image_compareworks. Very happy if there's some Stansby pytest/mpl knowledge™️ that helps here.