Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TST: Add tests for Affine2D #27511

Merged
merged 1 commit into from
Dec 14, 2023
Merged

TST: Add tests for Affine2D #27511

merged 1 commit into from
Dec 14, 2023

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Dec 13, 2023

PR summary

This is implicitly tested via all plots and layout. However, this makes it difficult to work on the transform stack directly (cf #22191), which these tests rectify.

I deliberately chose points/angles that were easy to work out in my head, so hopefully there are not any edge cases that were missed here. (Except for skew, for which I don't have an inherent idea of what that transform looks like, so I admit to just copy-pasting the existing output there.)

PR checklist

Comment on lines 104 to 105
trans_rad = Affine2D().skew(np.pi / 12, np.pi / 12)
trans_deg = Affine2D().skew_deg(15, 15)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking a bit more, this should probably skew different angles for x and y.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked with @ksunden and decided to use a couple of back-calculated values that produce a reasonable set of results.

This is implicitly tested via all plots and layout. However, this makes
it difficult to work on the transform stack directly, which these tests
rectify.
Copy link
Contributor

@oscargus oscargus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One may consider using match for the error tests, but that was not present in the earlier code anyway, so no point in requiring it here.

@oscargus oscargus merged commit e90058d into matplotlib:main Dec 14, 2023
42 checks passed
@QuLogic QuLogic deleted the affine-tests branch December 14, 2023 08:20
@QuLogic QuLogic added this to the v3.9.0 milestone Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants