FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add tests for mpl_toolkit anchored artists by oscargus · Pull Request #23863 · matplotlib/matplotlib · GitHub

Repository navigation

Add tests for mpl_toolkit anchored artists - #23863

Merged
ksunden merged 1 commit into
matplotlib:mainfrom
oscargus:testanchoredartists
Mar 13, 2023
Merged

ksunden merged 1 commit into
matplotlib:mainfrom
oscargus:testanchoredartists

Conversation

oscargus commented Sep 11, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

PR Summary

Tests for previously untested anchored artists.

Includes the fix in #23862 so if this is merged before that, no need for that. But since that should go in 3.6 and this doesn't have to...

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

jklymak commented Sep 11, 2022

Copy link
Copy Markdown
Member

We should probably talk about where all these tests are going? mpl_toolkits has, in the past, been discussed as under-tested and under-maintained. If we want to maintain it, why not move into the main library?

Copy link
Copy Markdown
Member Author

I have not really understood the role of mpl_toolkits. Is it correct that it is possible to "install" other packages under mpl_toolkits and that some do?`

Primarily just trying to increase coverage and learn a bit more of the code base... (And sometimes, untested code actually turns out to be incorrect, like in this case.)

But one could for sure, I guess, claim that for 4.0 one moves the useful parts of it to the main library?

jklymak commented Sep 11, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

💯 your work here has been great and, if we plan to keep these routines around, is well overdue. But if the plan is to obsolete a fair bit of this, then I didn't want you to feel you had wasted effort.

It would be great to revisit this on a call soon. I guess id recommend that if you find parts of the tool kit useful, consider if there are straightforward ways to bring into the main library. For instance we did that with inset_axes a couple of years ago, and 3.6 will have layout="compressed" which is meant to do the same things as image_grid.

Copy link
Copy Markdown
Member Author

I do not really have that much of a threshold for doing this. Something I play around with when I lack energy for proper things... It will probably take some time to deprecate etc anyway.

jklymak commented Sep 12, 2022

Copy link
Copy Markdown
Member

Yes but if we codify the behaviour with tests, it becomes even harder to change and or deprecate.

oscargus commented Sep 12, 2022 via email •
edited
Loading

Copy link
Copy Markdown
Member Author

QuLogic added this to the v3.7.2 milestone Mar 8, 2023
ksunden merged commit bc28efb into matplotlib:main Mar 13, 2023
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Mar 13, 2023
rcomer added a commit that referenced this pull request Mar 13, 2023
…863-on-v3.7.x

Backport PR #23863 on branch v3.7.x (Add tests for mpl_toolkit anchored artists)
oscargus deleted the testanchoredartists branch March 13, 2023 22:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

4 participants


Back | FazBrowse Home | New Git URL