Changes for `matplotlib3.8` #339

AdamOrmondroyd · 2023-09-15T23:46:46Z

Description

matplotlib3.8 released yesterday morning.

Axes._get_lines.prop_cycler has been removed, instead use Axes._get_lines.get_next_color() (inevitable with private member usage)
plt.hexbin now plots bins ≥mincnt, not >mincnt, which changes the behaviour with the default of 0 (corresponding to None in the signature). I have used np.finfo.tiny to correct this.
tcolors is deprecated, I have lifted the code used to simulate it in matplotlib to restore the test in test_contour_plot_2d().
samples.plot.[fast]kde_2d() now only adds two collections to the axes
plt.contour colors are out of whack (one weird extra one and the others are slightly different)

The last three are all symptoms of plt.contour returning QuadContourSet (one per level, so usually two for a contour plot) rather than PathCollection.

I'm completely stumped by the last one.

Fixes # (issue)

Checklist:

I have performed a self-review of my own code
My code is PEP8 compliant (flake8 anesthetic tests)
My code contains compliant docstrings (pydocstyle --convention=numpy anesthetic)
New and existing unit tests pass locally with my changes (python -m pytest)
I have added tests that prove my fix is effective or that my feature works
I have appropriately incremented the semantic version number in both README.rst and anesthetic/_version.py

…re also different (remember to attach screenshot to PR

AdamOrmondroyd · 2023-09-15T23:55:51Z

tests/test_samples.py

@@ -1245,7 +1245,7 @@ def test_samples_dot_plot():
    assert len(axes) == 2

    axes = samples.plot.kde_2d('x0', 'x1')
-    assert len(axes.collections) == 5
+    assert len(axes.collections) == 2


This test will fail on mpl<3.8. Is this a test we really need to do? The number of collections feels like matplotlib just doing matplotlib things

I agree (although 2 makes more sense here than 5...). Is there anything explicit we can test to confirm that it has plotted something?

Check it's more than 0?

That would do

AdamOrmondroyd · 2023-09-15T23:56:42Z

anesthetic/plotting/_matplotlib/core.py

@@ -131,6 +131,8 @@ def __init__(self, data, x, y, C=None, **kwargs) -> None:
            C = '__weights'
            data[C] = data.get_weights()
            kwargs['reduce_C_function'] = np.sum
+            if 'mincnt' not in kwargs:
+                kwargs['mincnt'] = np.finfo(np.float64).tiny


alias for smallest_normal but I like tiny

codecov · 2023-09-16T10:50:45Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (8563cae) 100.00% compared to head (9035297) 100.00%.

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #339   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           34        34           
  Lines         2868      2868           
=========================================
  Hits          2868      2868

Files	Coverage Δ
anesthetic/_version.py	`100.00% <100.00%> (ø)`
anesthetic/plot.py	`100.00% <100.00%> (ø)`
anesthetic/plotting/_matplotlib/core.py	`100.00% <100.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

AdamOrmondroyd · 2023-09-26T17:41:32Z

An extra level of .collections[0] restores the original test, as the first gets at the QuadContour object, and the second gets us to the deprecated API we were using to test before.

The plots at least look identical to the previous matplotlib

AdamOrmondroyd · 2023-09-30T09:53:09Z

Still stumped 😢 using the old api will be deprecated in a couple of minor versions so I'd prefer not to rely on it.

I just can't work out what the five colours from QuadContourSet.get_edgecolor() actually correspond to

lukashergt · 2023-10-01T02:32:27Z

Still stumped 😢 using the old api will be deprecated in a couple of minor versions so I'd prefer not to rely on it.

Agreed, how about we add a matplotlib version check to the tests and test both old and new API. In the actual code I'd find that really annoying, but if we limit it to the tests that should be ok, right?

AdamOrmondroyd · 2023-10-01T08:58:23Z

Still stumped 😢 using the old api will be deprecated in a couple of minor versions so I'd prefer not to rely on it.

Agreed, how about we add a matplotlib version check to the tests and test both old and new API. In the actual code I'd find that really annoying, but if we limit it to the tests that should be ok, right?

But what do we do in two matplotlib's time? My concern is I don't understand the QuadContourSet colours

lukashergt · 2023-10-01T09:12:49Z

But what do we do in two matplotlib's time?

Keep adding to the tests and we could say anything older than 2 or 3 minor matplotlib versions get's scratched.

My concern is I don't understand the QuadContourSet colours

QuadContourSet.get_edgecolor() should return the colours of the contour lines (so it should be one more than the number of levels). QuadContourSet.get_facecolor() should return the colours of the contourf faces (as many as there are levels). In 3.7 each face and each line was its own PathCollection (that's where the number 5 came from, 2 faces and 3 lines). Now in 3.8 it seems these have been grouped into two QuadContourSet, one for contourf and one for contour.

AdamOrmondroyd · 2023-10-01T10:08:18Z

got it, thanks Lukas 😄 hopefully that's this PR done

lukashergt

Thanks a bunch @AdamOrmondroyd! Please squash and merge.

AdamOrmondroyd · 2023-10-01T20:55:47Z

Whoops! Left that in from while I was remembering how to check the version

AdamOrmondroyd added 6 commits September 15, 2023 18:16

first pass at fixing next color (could be wrong color)

f7c3a20

kde_2d seems to now be made up of 4 collections, instead of 5. They a…

b09a15a

…re also different (remember to attach screenshot to PR

use set_cmap. Side-effect is now only two collections

e79910c

work around QuadContourSet.tcolors deprecation:

031747a

matplotlib now plots >=mincnt, not >mincnt

b7053a6

version bump

94ada4d

AdamOrmondroyd commented Sep 15, 2023

View reviewed changes

just test for more than 0 collections

65477bc

This was referenced Sep 20, 2023

matplotlib 3.8 + minimum dependencies check #340

Closed

Ignore "You are using [hist/kde] as a plot kind" warnings #342

Merged

lukashergt mentioned this pull request Sep 21, 2023

Restrict matplotlib < 3.8 #341

Merged

6 tasks

AdamOrmondroyd mentioned this pull request Sep 22, 2023

Restrict upper limit of pandas and matplotlib + "use latest versions" test #343

Closed

AdamOrmondroyd added 4 commits September 22, 2023 18:46

Merge branch 'master' into mpl38

ab7039b

version bump

27874f7

unlimit matplotlib version from merge

65138ec

deprecated solution to contour test :(

2f54994

Merge branch 'master' into mpl38

e0c9587

finally clicked what's going on - different tests for mpl>3.8

a2a72e4

AdamOrmondroyd marked this pull request as ready for review October 1, 2023 10:25

AdamOrmondroyd requested a review from lukashergt October 1, 2023 10:25

AdamOrmondroyd mentioned this pull request Oct 1, 2023

Test latest dependencies #344

Merged

6 tasks

clean up unnecessary pass

9035297

lukashergt approved these changes Oct 1, 2023

View reviewed changes

AdamOrmondroyd merged commit 434d97e into handley-lab:master Oct 1, 2023
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes for `matplotlib3.8` #339

Changes for `matplotlib3.8` #339

AdamOrmondroyd commented Sep 15, 2023 •

edited

Loading

AdamOrmondroyd Sep 15, 2023

williamjameshandley Sep 16, 2023

AdamOrmondroyd Sep 16, 2023

williamjameshandley Sep 16, 2023

AdamOrmondroyd Sep 15, 2023

codecov bot commented Sep 16, 2023 •

edited

Loading

AdamOrmondroyd commented Sep 26, 2023

AdamOrmondroyd commented Sep 30, 2023

lukashergt commented Oct 1, 2023

AdamOrmondroyd commented Oct 1, 2023

lukashergt commented Oct 1, 2023

AdamOrmondroyd commented Oct 1, 2023

lukashergt left a comment

AdamOrmondroyd commented Oct 1, 2023

Changes for matplotlib3.8 #339

Changes for matplotlib3.8 #339

Conversation

AdamOrmondroyd commented Sep 15, 2023 • edited Loading

Description

Checklist:

AdamOrmondroyd Sep 15, 2023

Choose a reason for hiding this comment

williamjameshandley Sep 16, 2023

Choose a reason for hiding this comment

AdamOrmondroyd Sep 16, 2023

Choose a reason for hiding this comment

williamjameshandley Sep 16, 2023

Choose a reason for hiding this comment

AdamOrmondroyd Sep 15, 2023

Choose a reason for hiding this comment

codecov bot commented Sep 16, 2023 • edited Loading

Codecov Report

AdamOrmondroyd commented Sep 26, 2023

AdamOrmondroyd commented Sep 30, 2023

lukashergt commented Oct 1, 2023

AdamOrmondroyd commented Oct 1, 2023

lukashergt commented Oct 1, 2023

AdamOrmondroyd commented Oct 1, 2023

lukashergt left a comment

Choose a reason for hiding this comment

AdamOrmondroyd commented Oct 1, 2023

Changes for `matplotlib3.8` #339

Changes for `matplotlib3.8` #339

AdamOrmondroyd commented Sep 15, 2023 •

edited

Loading

codecov bot commented Sep 16, 2023 •

edited

Loading