ENH: include property name in artist AttributeError#28573
Conversation
| with pytest.raises(AttributeError) as exec_info: | ||
| ax.voxels(filled=filled, x=x, y=y, z=z) | ||
| assert exec_info.value.name == 'x' |
There was a problem hiding this comment.
This was the only test I found that covers the changed code. Should I add something new or is this enough?
There was a problem hiding this comment.
Take or leave my optional suggestions. I don’t think this needs additional tests.
| # x, y, z are positional only - this passes them on as attributes of | ||
| # Poly3DCollection | ||
| with pytest.raises(AttributeError): | ||
| with pytest.raises(AttributeError) as exec_info: |
There was a problem hiding this comment.
Maybe add a match parameter to check that the name is in the message.
| if not callable(func): | ||
| raise AttributeError( | ||
| errfmt.format(cls=type(self), prop_name=k)) | ||
| errfmt.format(cls=type(self), prop_name=k), |
There was a problem hiding this comment.
The docstring could mention that formatting happens via named placeholders {cls} and {prop_name}.
story645
left a comment
There was a problem hiding this comment.
I think you should add a what's new cause that's probably the easiest way to tell folks "hey better error messages" and you've basically already written it in the PR body, so I'm gonna leave this for you to merge if you wanna add one (or not).
|
Added a whatsnew. Is there a better directive to use for the output block? |
PR summary
Closes #27878
Add a
nameto the exception that is raised when an invalid keyword parameter is passed. This helps downstream packages who wrap our functions with their exception handling:We could also add the artist type as the
objattribute, or we could wait till someone asks for that.Should this get a whatsnew entry?
PR checklist