Skip to content

MNT: Add test for aitoff-projection #15092

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

Merged
merged 1 commit into from
Aug 22, 2019

Conversation

ImportanceOfBeingErnest
Copy link
Member

@ImportanceOfBeingErnest ImportanceOfBeingErnest commented Aug 20, 2019

PR Summary

Closes #14503

Adds test for fix in #14451

Unfortunately, the initial problem occured at draw time, so one will need to use an image comparison test.

Running the test in matplotlib 3.1 results in

image

Running the test in current master results in

image

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

@ImportanceOfBeingErnest ImportanceOfBeingErnest added this to the v3.2.0 milestone Aug 20, 2019
https://github.com/matplotlib/matplotlib/pull/14451
"""
np.random.seed(42)
ra_random = np.random.rand(20) * 2 * np.pi
Copy link
Member

Choose a reason for hiding this comment

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

Can these be a structured grid (or line of points) instead of random points? That way it's easier to understand changes to the positioning of the points due to any future code changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, changed to use a grid. See updated post above.


fig, ax = plt.subplots(figsize=(8, 4.2),
subplot_kw=dict(projection="aitoff"))
ax.tick_params(left=False, bottom=False,
Copy link
Member

Choose a reason for hiding this comment

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

Why remove the ticks?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because they aren't relevant for what's being tested. Would you like me to include them?

Copy link
Member

Choose a reason for hiding this comment

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

If we add them then the image is also testing the tick layout, and will be a good catch of any changes in ticking changes for this projection.

Copy link
Member Author

Choose a reason for hiding this comment

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

aitoff projection doesn't have ticks, and the labels are excluded by the remove_text=True option in the @image_comparison decorator (I cannot get the matching freetype to install on my system). But I went to put in a grid now, which would test the tick locations.

@jklymak jklymak merged commit a0b8cae into matplotlib:master Aug 22, 2019
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.

Add a test for #14451
4 participants