Fix broken scatter/fill_between in log-scale axis

[lucasb-eyer]

I did try to fix this myself, but failed. So I asked Codex (gpt5-high) and it found the issue and proposed this fix, which I verified works, and I streamlined its code a bit.

Here's what it had to say:

• mplexporter/mplexporter/exporter.py:6-275 now imports numpy/mpath and routes collections through _prepare_collection_data, which preserves data-space vertices/offsets whenever a non-affine ax.transData branch is
  involved (log scales, semilog, etc.). As a result, fill_between, scatter, and other PathCollections on log axes export with pathcoordinates/offsetcoordinates == 'data', so the rendered polygons obey zooming/panning.
• mpld3/tests/test_elements.py:76-103 adds regression tests for both fill_between and scatter on logarithmic axes to guard against future regressions in the exported coordinate system.

Here's an example of my app before this fix:

And after this fix:

[lucasb-eyer]

PS: this is the test it's talking about adding to mpld3 repo. I would open a PR there which also moves the submodule, if you agree this is meaningful:

diff --git a/mpld3/tests/test_elements.py b/mpld3/tests/test_elements.py
index 8a89772..dbce6bf 100644
--- a/mpld3/tests/test_elements.py
+++ b/mpld3/tests/test_elements.py
@@ -76,6 +76,29 @@ def test_scatter():
                  [[7.607257743127308, 0.0, 0.0, 7.607257743127308, 0.0, 0.0]])
 
 
+def test_fill_between_logscale():
+    fig, ax = plt.subplots()
+    ax.set_xscale('log')
+    ax.set_yscale('log')
+    x = np.logspace(0, 2, 5)
+    ax.fill_between(x, x, x * 2)
+    rep = fig_to_dict(fig)
+    coll = rep['axes'][0]['collections'][0]
+
+    assert_equal(coll['pathcoordinates'], 'data')
+
+
+def test_scatter_logscale():
+    fig, ax = plt.subplots()
+    ax.set_xscale('log')
+    ax.set_yscale('log')
+    ax.scatter([1, 10], [1, 100])
+    rep = fig_to_dict(fig)
+    coll = rep['axes'][0]['collections'][0]
+
+    assert_equal(coll['offsetcoordinates'], 'data')
+
+
 def test_patch():
     fig, ax = plt.subplots()
     ax.add_patch(plt.Rectangle((0, 0), 1, 2, alpha=0.2, linewidth=2,

[lucasb-eyer]

I think this might close multiple issues in mpld3 proper, but I am not going to test each of them, of course:

mpld3/mpld3#488
mpld3/mpld3#227
mpld3/mpld3#487

[vladh]

Thanks for this! This looks good at first sight, but I need to do some more in-depth review, because (1) AI, (2) I want to make sure I actually understand the change well, and (3) the code currently is named to coincide with the original matplotlib function and argument names, which isn't necessary. Will get back to you. 😊

[lucasb-eyer]

1. Heh, I guessed so which is why I even mentioned it, but just FYI: I did carefully review it, but AI has the whole matplotlib internals in its head and I dont. From old issues, it looks like even Jake had no idea on why the issue exists :)
2. That's a change I did on top of AI intentionally - you can now easily diff it with the matplotlib funtion to see the actual change (which I did to review it), and if matplotlib ever changes that function, we can easily adopt the changes.

PS: I will have a couple more such AI-assisted fixes coming in the near future as I go through missing features for my use-case. All of them are open issues, and I'll always disclose when I use AI.

[aflaxman]

I think this is so cool! AI-assisted coding could be a really good match for this project. I did so (AI-assisted) review of this PR and I can confirm that it works. You can find my notes here.

[vladh]

Just checking in to say:

• Hey @aflaxman, great to see you here! Feel free to review and merge things yourself if you'd like.
• Feel free to send a PR to add contributions from your mpld3 notes to CONTRIBUTING.md and potentially RELEASING.md. In particular, adding uv notes to these documents would be great.
• I'm trying to make time to review PRs in the next week or so

[lucasb-eyer]

I think this is so cool! AI-assisted coding could be a really good match for this project. I did so (AI-assisted) review of this PR and I can confirm that it works. You can find my notes here.

Yes that was my thought exactly! And it's become really good at figuring things out! Still needs close review, and sometimes nudging away from an over-complicated solution to a simpler one, but generally, I was able to fix all things missing for my use-case, which without AI I simply would never have succeeded in.

The submodule setup is getting in the way a little though, requires a lot of manual fiddling, and makes having a whole bunch of PRs stacked up like I have very cumbersome. But I'm not sure I have a better idea. May ask LLM about it :)

[vladh]

I don't like the submodule either. Probably the best solution IMO is to migrate to uv, then install mplexporter as a Python package from a git URL.

[lucasb-eyer]

Feel free to review and merge things yourself if you'd like.

But, do note that more than half of my PRs still need the "manual update" of fixing the right submodule commit (and need the mplexporter part merged first for that, IIUC), as well as re-building the js bundles. I will do so for each once it's LGTM'd, didn't want to waste time in case a PR needs more work or is rejected. Or you could do so too, ofc, just noting that in most of my PRs you wouldn't want to hit merge as-is.

[vladh]

It might be best to review + merge the mplexporter stuff first, to avoid any confusion.

[lucasb-eyer]

About half of my PRs are pairs with one here and one in mpld3 going hand-in-hand. Each time this is the case, I cross-link them. (and the other half are mpld3-only).

[aflaxman]

I don't like the submodule either. Probably the best solution IMO is to migrate to uv, then install mplexporter as a Python package from a git URL.

If I remember correctly, the design with mplexporter in a separate python module goes back to an idea @jakevdp had that there might be multiple uses for this sort of exported mpl data beyond just the mpld3 package. I don't think that ever happened, though, and so at this point perhaps it would be acceptable to merge the repos so that we can make all the changes in one place.

It has been a long time since I've thought about this, though, so I will be the first to acknowledge that I very well could be missing something here!

[vladh]

I'm happy to just merge mplexporter into mpld3. We can still continue to maintain it as a “public” API, so if we need to split it out later, it will be trivial.

[lucasb-eyer]

I was wondering about the "this never happened" and did a little search.
I found a few big repos which forked it into their own repo many many years ago, so shouldn't affect us, for example:

• https://github.com/plotly/plotly.py/tree/main/plotly/matplotlylib/mplexporter
• https://github.com/mrocklin/bokeh/tree/master/bokeh/compat/mplexporter

Other than that, here's all I could find, which is a few repos here and there: https://github.com/search?q=%22import+mplexporter%22+OR+%22from+mplexporter%22&type=code

So, I can see both pros and cons, tbh. Maybe we just live with the submodule, idk.