Add widths, heights and angles setter to EllipseCollection

[ericpre]

PR summary

Currently, the following example:

import matplotlib.pyplot as plt
from matplotlib.collections import EllipseCollection
import numpy as np

rng = np.random.default_rng(0)

widths = (2, )
heights = (3, )
angles = (45, )
offsets = rng.random((10, 2)) * 10

fig, ax = plt.subplots()

ec = EllipseCollection(
    widths=widths,
    heights=heights,
    angles=angles,
    offsets=offsets,
    units='x', 
    offset_transform=ax.transData,
    )

ax.add_collection(ec)
ax.set_xlim(-2, 12)
ax.set_ylim(-2, 12)

new_widths = rng.random((10, 2)) * 2
new_heights = rng.random((10, 2)) * 3
new_angles = rng.random((10, 2)) * 180

ec.set(widths=new_widths, heights=new_heights, angles=new_angles)

will fails with the errors:

  File ~\Dev\matplotlib\lib\matplotlib\artist.py:147 in <lambda>
    cls.set = lambda self, **kwargs: Artist.set(self, **kwargs)

  File ~\Dev\matplotlib\lib\matplotlib\artist.py:1221 in set
    return self._internal_update(cbook.normalize_kwargs(kwargs, self))

  File ~\Dev\matplotlib\lib\matplotlib\artist.py:1213 in _internal_update
    return self._update_props(

  File ~\Dev\matplotlib\lib\matplotlib\artist.py:1187 in _update_props
    raise AttributeError(

AttributeError: EllipseCollection.set() got an unexpected keyword argument 'widths'

PR checklist

• [n/a] "closes #0000" is in the body of the PR description to link the related issue
• new and changed code is tested
• [n/a] Plotting related features are demonstrated in an example
• New Features and API Changes are noted with a directive and release note
• Documentation complies with general and docstring guidelines

[oscargus]

It probably makes sense to use the newly added methods in the constructor.

(Are there getters as well?)

[ericpre]

Thank you @oscargus for the review, this should be all done.

(Are there getters as well?)

I didn't add the getters because we don't need them and I don't know how to get their value after the transform. But if someone give me some pointers, I am happy to do that in this PR - if there are setters, it would be fair to expect getters too!

[oscargus]

I cannot help with the getters, unfortunately. While it is indeed ideal to have both, it is clearly better to have only setters compared to nothing.

[tacaswell]

We talked about this on the call and are 👍🏻 on it going in but would like getters.

The consensus on the call was for the getters to undo the geometric transformations, but don't worry about the ravel (if someone is passing in higher than 1D data, we would like to know why before bending over backwards to give it back).

[ericpre]

I added the getters which return the same as what is given to the setters and rebased. The CI failures shouldn't be related to the changes of this PR.

[timhoffm]

In a similar case #26410 (comment) where multiple related arrays can be set, we advocated for a central function that can take one or multiple of the arrays and make sure the final data (mixture of existing and new arrays) has consistent array lengths.

I suggest that we follow this approach here as well - at least for the geometry parameters widths, heights, angles, offsets. I grant that we won't check other potentially sequence-like parameters (e.g. linewidths), but AFAIK they are just cycled continuously and don't have to match in lengths.

[QuLogic]

A friendly ping on this; I'd like to put out 3.9 beta/rc next week.

[ericpre]

Thanks @QuLogic, yes it would be good to finish this PR.

@timhoffm, does it sound good to overwrite the set method? In the example added in this PR, set is used these values in a single call. This has the advantage of keeping the API minimal and consistent.

[timhoffm]

@timhoffm, does it sound good to overwrite the set method?

That may work, but it's a bit tricky because the set methods are auto-generated to include all properties of the class:

matplotlib/lib/matplotlib/artist.py

Lines 139 to 150 in 9e18a34

	if not hasattr(cls.set, '_autogenerated_signature'):
	# Don't overwrite cls.set if the subclass or one of its parents
	# has defined a set method set itself.
	# If there was no explicit definition, cls.set is inherited from
	# the hierarchy of auto-generated set methods, which hold the
	# flag _autogenerated_signature.
	return
	cls.set = lambda self, **kwargs: Artist.set(self, **kwargs)
	cls.set.__name__ = "set"
	cls.set.__qualname__ = f"{cls.__qualname__}.set"
	cls._update_set_signature_and_docstring()

You may have to care for _autogenerated_signature and/or _update_set_signature_and_docstring() to get explicit set signature and docstring without manually having to specify all parameters. This is some you would have to fiddle into.

---

Edit: I see this is slightly more complicated than #26410 (comment). I thought we make an equivalent to set_XYUVC(), but that would be something like set_widths_heights_angles_offsets() which is quite bulky, and I don't believe this would genralize well to arbitrary collections.
Thus, I'm not sure we get a viable solution with length consistency checks quickly. Therefore, if people want the fundamental functionality to set these parameters (without saftety checks) in 3.9, I would be ok to merge the current proposal as is. It's not adding any API that we'd regret later.

[ericpre]

I added a _check_length method to check the length of the input parameter using offsets as reference for the number of items in the collection.

[ericpre]

Similarly as in #26410, I remove checking the length of the parameters to leave for another PR.

[QuLogic]

Do we need to check internals now that we have the getters?

[ericpre]

These check that the internal values are correct (multiplication of 0.5) and these are different from the values returned by the getter, so I would say that this is still worth checking these values?

[timhoffm]

Testing internals is not ideal, but OTOH these tests do not impose a big liability. It's unlikely that we will refactor the internals and if so, the tests can be easily adapted.

Alternatives would be an image comparison (not favored) or checking the bounding box of an EllipseCollection with a single entry.

[ericpre]

If this is find to keep to check as it is I would prefer that option because it is straighforward and more simple than the bounding box alternative.

[ericpre]

Thanks @QuLogic, I have addressed your comments!

[ericpre]

These check that the internal values are correct (multiplication of 0.5) and these are different from the values returned by the getter, so I would say that this is still worth checking these values?

[timhoffm]

Take or leave the internal check.

[QuLogic]

I think we can ignore the AppVeyor failure here, as it seems to be something to do with wxPython.