Use Path.arc() to interpolate polar arcs. #14852

anntzer · 2019-07-18T17:46:04Z

Currently, in polar plots, arcs are rendered by linearly interpolating
in (theta, r) space before conversion to (x, y) space (if their
_interpolation_steps attribute is >1 -- this attribute is described in
the docs as "an implementation detail not intended for public use").

When _interpolation_steps == 1, this PR doesn't change anything -- there
is an example (specialty_plots/radar_chart) that explicitly relies on
being able to set _interpolation_steps to 1 and get a "polygonal" polar
axes rather than a circular one.

When _interpolation_steps > 1, however, this PR looks for paths segments
that are either at a constant theta or a constant r. It transforms
constant-theta segments to a single segment (because interpolation
doesn't help there) and transforms constant-r segments (circular arcs)
to a Bezier approximation of a circle which is already implemented in
Path.arc()).

This greatly decreases the number of control points for such plots in
vector outputs (which are thus smaller -- note the line counts changes
in the svg files...), and also improves rasterization by mplcairo (cairo
appears to be not so good at filling areas with a lot of close vertices).

The many changes in baseline images are due to, well, the change of
approximation for drawing circles. (Note that if we agree with the
approach of this PR, I could perhaps do a first PR to decrease the
number of polar tests so that I don't have to commit so many new
images...)

Example: polar(); bar([0], [0.9])

matplotlib svg before, showing control points in inkscape:

matplotlib svg after, showing control points in inkscape:

mplcairo before:

mplcairo after:

PR Summary

PR Checklist

Has Pytest style unit tests
Code is Flake 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

jklymak · 2019-07-18T19:11:56Z

This looks really useful. Can the old images be kept by setting interpolation_steps=1? OTOH I think if a default changes we should consider buying the bullet and changing the test images. Same for the image antialiasing PR

anntzer · 2019-07-18T22:32:31Z

No, interpolation_steps=1 results in something like https://matplotlib.org/3.1.0/gallery/specialty_plots/radar_chart.html.

I'm fine with modifying a few baseline images but I think it's also a good opportunity to see whether we can combine some tests...

QuLogic

Well, I like the idea, but I won't approve since you want to work on thinning out the test images. Also, should try and get these new bits below to be tested.

QuLogic · 2019-07-25T08:43:07Z

lib/matplotlib/projections/polar.py

+                             * self._axis.get_rsign())
+                    if last_td <= td:
+                        while td - last_td > 360:
+                            arc = Path.arc(last_td, last_td + 360)


This block seems untested.

test added, although it's not particularly interesting (I can't actually think of a very interesting test for that...)

QuLogic · 2019-07-25T08:43:26Z

lib/matplotlib/projections/polar.py

+                        # The reverse version also relies on the fact that all
+                        # codes but the first one are the same.
+                        while last_td - td > 360:
+                            arc = Path.arc(last_td - 360, last_td)


As does this block.

same as above

QuLogic · 2019-07-25T08:44:07Z

lib/matplotlib/projections/polar.py

+                        xys.extend(arc.vertices[::-1][1:] * r)
+                        codes.extend(arc.codes[1:])
+                else:  # Interpolate.
+                    trs = cbook.simple_linear_interpolation(


And this block too.

QuLogic · 2019-07-25T08:55:09Z

lib/matplotlib/projections/polar.py

+                        np.column_stack([(last_t, last_r), trs]),
+                        path._interpolation_steps)[1:]
+                    xys.extend(self.transform_non_affine(trs))
+                    codes.extend(Path.LINETO for _ in trs)


Is this really better than codes.extend([Path.LINETO] * len(trs))?

either way works for me, switched to your preference.

anntzer · 2019-07-25T13:34:01Z

Rebased on top of #14888 which deletes a few baseline images (replacing them by check_figures_equal).

anntzer · 2019-07-25T13:44:20Z

Also, note that with this patch, one cannot anymore set e.g. GRIDLINE_INTERPOLATION_STEPS to a small but >1 number (e.g. 3) and see 3-point interpolation on circular arcs, but I'm not too worried about that...

timhoffm · 2019-07-27T09:14:33Z

I'm positive on the idea of this change. Ping when you think it's ready for review.

anntzer · 2019-07-27T11:29:52Z

This PR includes #14888 so it can already be reviewed (at worst this one gets merged before #14888 and both commits go in at the same time). (I don't think the other polar baseline images can "easily" be removed.)

QuLogic · 2019-09-08T04:26:18Z

Docs fail, might need a rebase.

anntzer · 2019-09-08T08:39:19Z

rebased

Currently, in polar plots, arcs are rendered by linearly interpolating in (theta, r) space before conversion to (x, y) space (if their _interpolation_steps attribute is >1 -- this attribute is described in the docs as "an implementation detail not intended for public use"). When _interpolation_steps == 1, this PR doesn't change anything -- there is an example (specialty_plots/radar_chart) that explicitly relies on being able to set _interpolation_steps to 1 and get a "polygonal" polar axes rather than a circular one. When _interpolation_steps > 1, however, this PR looks for paths segments that are either at a constant theta or a constant r. It transforms constant-theta segments to a single segment (because interpolation doesn't help there) and transforms constant-r segments (circular arcs) to a Bezier approximation of a circle which is already implemented in Path.arc()). This greatly decreases the number of control points for such plots in vector outputs (which are thus smaller), and also improves rasterization by mplcairo (cairo appears to be not so good at filling areas with a lot of close vertices). The many changes in baseline images are due to, well, the change of approximation for drawing circles.

QuLogic

Bit unfortunate about all the test images, I guess.

anntzer · 2020-03-13T09:23:23Z

I don't mind waiting to see at least whether the test-images-infrastructure-rewrite GSoC will happen or not (after all this has already been there for 8 months, an extra couple of them isn't too bad...).

tacaswell · 2020-03-13T18:25:27Z

Decided to merge as-is as this is a pretty big improvement for SVG and mplcairo.

anntzer added the topic: polar label Jul 18, 2019

QuLogic reviewed Jul 25, 2019

View reviewed changes

anntzer mentioned this pull request Jul 25, 2019

Replace some polar baseline images by check_figures_equal. #14888

Merged

6 tasks

anntzer force-pushed the polarinterp branch from 71cf4b5 to 89ffe8c Compare July 25, 2019 13:34

anntzer force-pushed the polarinterp branch from 89ffe8c to e956ad1 Compare July 25, 2019 13:57

jklymak requested a review from QuLogic July 26, 2019 23:15

anntzer force-pushed the polarinterp branch 2 times, most recently from 2701391 to 3e74404 Compare July 28, 2019 21:45

tacaswell added this to the v3.3.0 milestone Aug 12, 2019

anntzer force-pushed the polarinterp branch 2 times, most recently from 0842a13 to 851d81a Compare August 12, 2019 21:25

anntzer force-pushed the polarinterp branch from 851d81a to 31133ea Compare September 8, 2019 08:39

QuLogic added the status: needs rebase label Mar 7, 2020

anntzer force-pushed the polarinterp branch from 31133ea to 73a763a Compare March 9, 2020 11:08

anntzer removed the status: needs rebase label Mar 9, 2020

QuLogic approved these changes Mar 13, 2020

View reviewed changes

tacaswell merged commit 4331c53 into matplotlib:master Mar 13, 2020

anntzer deleted the polarinterp branch March 13, 2020 20:35

anntzer mentioned this pull request May 15, 2020

[feature request] circle -> SVG <circle> #17424

Open

anntzer mentioned this pull request Apr 16, 2021

Radar Chart with Polygonal Grid Lines #19981

Closed

This was referenced Aug 21, 2022

Polar errcaps #23592

Merged

Document polar handling of _interpolation_steps. #23695

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Path.arc() to interpolate polar arcs. #14852

Use Path.arc() to interpolate polar arcs. #14852

anntzer commented Jul 18, 2019

jklymak commented Jul 18, 2019

anntzer commented Jul 18, 2019

QuLogic left a comment

QuLogic Jul 25, 2019

anntzer Jul 25, 2019

QuLogic Jul 25, 2019

anntzer Jul 25, 2019

QuLogic Jul 25, 2019

anntzer Jul 25, 2019

QuLogic Jul 25, 2019

anntzer Jul 25, 2019

anntzer commented Jul 25, 2019

anntzer commented Jul 25, 2019

timhoffm commented Jul 27, 2019

anntzer commented Jul 27, 2019

QuLogic commented Sep 8, 2019

anntzer commented Sep 8, 2019

QuLogic left a comment

anntzer commented Mar 13, 2020

tacaswell commented Mar 13, 2020

Use Path.arc() to interpolate polar arcs. #14852

Use Path.arc() to interpolate polar arcs. #14852

Conversation

anntzer commented Jul 18, 2019

PR Summary

PR Checklist

jklymak commented Jul 18, 2019

anntzer commented Jul 18, 2019

QuLogic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer commented Jul 25, 2019

anntzer commented Jul 25, 2019

timhoffm commented Jul 27, 2019

anntzer commented Jul 27, 2019

QuLogic commented Sep 8, 2019

anntzer commented Sep 8, 2019

QuLogic left a comment

Choose a reason for hiding this comment

anntzer commented Mar 13, 2020

tacaswell commented Mar 13, 2020