Test to guard against styling side-effects. #160
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dstansby
reviewed
Jun 12, 2023
There was a problem hiding this comment.
How about a normal distribution? They're more aesthetically pleasing.
(Also perhaps a more intelligent test since I'm using the HistogramWidget.)
There was a problem hiding this comment.
my eyes were bleeding at the use of jet mainly, so the histogram is 👍 much better
dstansby
reviewed
Jun 12, 2023
|
If you like this, I'll force-push a squash to save the repo size.
|
dstansby
approved these changes
Jun 13, 2023
samcunliffe
force-pushed
the
guard-against-64
branch
from
f6ef269 to
6af9ce0
Compare
Use plt.subplots for safety. And no longer need copy.deepcopy. Save eyes and make a lovely histogram.
samcunliffe
force-pushed
the
guard-against-64
branch
from
6af9ce0 to
aede1f8
Compare
dstansby
approved these changes
Jun 13, 2023
I think the auto-merge is smart enough to disregard your approval because the commit hash changed (?) |
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 actually have 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.