Skip to content

Deprecated tight_bbox and tight_layout modules #22134

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 2 commits into from
Jan 9, 2022

Conversation

oscargus
Copy link
Member

@oscargus oscargus commented Jan 6, 2022

PR Summary

Related to #16181

The modules matplotlib.tight_bbox and matplotlib.tight_layout are deprecated.

Will add release notes once the tests pass (and I have a PR-number).

PR Checklist

Tests and Styling

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

Documentation

  • [N/A] New features are documented, with examples if plot related.
  • [N/A] 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).

@oscargus oscargus marked this pull request as draft January 6, 2022 17:42
@oscargus oscargus force-pushed the deprecate_tight branch 3 times, most recently from 71c59a7 to b368849 Compare January 6, 2022 18:35
@oscargus oscargus changed the title Deprecated tight_bbox and tight_layout modules Deprecated tight_bbox and tight_layout modules Jan 6, 2022
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

The doc build still fails with

/home/circleci/project/doc/api/prev_api_changes/api_changes_1.4.x.rst:152: WARNING: py:obj reference target not found: tight_bbox.adjust_bbox
/home/circleci/project/doc/api/prev_api_changes/api_changes_1.4.x.rst:152: WARNING: py:obj reference target not found: tight_bbox.process_figure_for_rasterizing
/home/circleci/project/doc/api/prev_api_changes/api_changes_2.2.0.rst:162: WARNING: py:obj reference target not found: tight_layout.auto_adjust_subplotpars
/home/circleci/project/doc/api/prev_api_changes/api_changes_3.0.1.rst:4: WARNING: py:obj reference target not found: tight_layout.auto_adjust_subplotpars
/home/circleci/project/doc/api/prev_api_changes/api_changes_3.0.1.rst:4: WARNING: py:obj reference target not found: tight_layout.get_tight_layout_figure

These historic API change notes reference the now non-public API. The way to handle this is to change the links to verbatim as that API is no longer public and available in the docs. E.g. for the first one, change

`.tight_bbox.adjust_bbox`

to

``matplotlib.tight_bbox.adjust_bbox``

@oscargus
Copy link
Member Author

oscargus commented Jan 6, 2022

Thanks! I had that question in #22133, so I'll fix both.

@timhoffm
Copy link
Member

timhoffm commented Jan 6, 2022

I did a specific comment in #22133 as well.

@oscargus
Copy link
Member Author

oscargus commented Jan 7, 2022

Thanks @timhoffm !

I've updated both and will set them to be ready when the tests pass.

I decided to not put links in the deprecation notices to not have to edit them when the modules actually are removed.

@oscargus oscargus marked this pull request as ready for review January 7, 2022 19:43
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

It just occured to me that we can an should keep the history for git blame. If you create a new _tight_bbox.py, copy everything over and add the stub in tight_bbox.py, then all of _tight_bbox.py will be marked as your change and not have any history behind that.

To preserve the history:

  1. git mv tight_bbox.py _tight_bbox.py and put this into a commit.
  2. Create a new tight_bbox.py with the deprecation and put this into a second commit.

It's important that the move is in a separate commit. Otherwise git assumes you created _tight_bbox.py and modified tight_bbox.py. However, you can have the moves of both files in one commit and both deprecations in the second.

Sorry to bring this in at a quite late review stage, but it's really important for us to be able to easily dig into the code history.

@oscargus
Copy link
Member Author

oscargus commented Jan 8, 2022

Sure! Shouldn't be much work! (And clearly makes sense, although I didn't think about it before.)

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.

3 participants