Skip to content

Expire deprecation of \mathcircled#16204

Merged
tacaswell merged 1 commit intomatplotlib:masterfrom
anntzer:unmathcircled
Feb 10, 2020
Merged

Expire deprecation of \mathcircled#16204
tacaswell merged 1 commit intomatplotlib:masterfrom
anntzer:unmathcircled

Conversation

@anntzer
Copy link
Copy Markdown
Contributor

@anntzer anntzer commented Jan 13, 2020 •
edited
Loading

PR Summary

A slightly tricky one: a bunch of baseline images had to be renumbered because the mathtext tests just index baseline images by their ordinal number, but the mathcircled tests were in the middle... (yes, this is brittle, but I'm not going to refactor this now)
1st commit removes the now unused baselines; 2nd commit renumbers the rest (so that the motion is more obvious in the log). (Github show them as changed binaries, but they're actually all just deletes and renames.)
attn @timhoffm this may have been one of the issue you ran into in #15890.

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

@anntzer anntzer added this to the v3.3.0 milestone Jan 13, 2020
This was referenced Jan 13, 2020
Merged
Closed
dstansby
dstansby approved these changes Feb 7, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

Needs a rebase

@anntzer anntzer force-pushed the unmathcircled branch 2 times, most recently from 1950b2c to 96465eb Compare February 7, 2020 10:30
@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Feb 7, 2020

thanks, done

tacaswell
tacaswell requested changes Feb 7, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

Have an idea of how to avoid re-naming all of the test images.

@tacaswell
Copy link
Copy Markdown
Member

tacaswell commented Feb 7, 2020

My approval should be suspect given that I pushed code to this branch, but 👍 to the removal.

I think it is better to accept a bit more complexity in generating the fixtures to avoid having to move many many files.

@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Feb 8, 2020 •
edited
Loading

Fixed a flake8 error, and additionally removed the corresponding entries in _mathtext_data.py.
Your strategy looks good to me, you can consider this as my approval of your patch :)

@tacaswell
Copy link
Copy Markdown
Member

tacaswell commented Feb 8, 2020

rebased to fix the deprecation conflicts, will merge when green.

@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Feb 8, 2020

(may be worth withholding merges until https://gitter.im/matplotlib/matplotlib?at=5e3e96051d23aa47aa003c3b is resolved (either way) in order to not make the history edit more complicated)

@tacaswell
Copy link
Copy Markdown
Member

tacaswell commented Feb 8, 2020

Oh good, UI is showing us all of the extra commits, will fix.

@tacaswell
Copy link
Copy Markdown
Member

tacaswell commented Feb 8, 2020

hmm, it is now not letting me force-push to this branch, sorry @anntzer

tacaswell
tacaswell requested changes Feb 8, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

needs to have bad commits removed, anyone can clear this and and anyone con merge on green.

@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Feb 8, 2020

rebased

@tacaswell
Copy link
Copy Markdown
Member

tacaswell commented Feb 9, 2020

of course I just broke this by merging the stackrel PR.

@anntzer
Expire deprecation of mathcircled.
6be45af 
Use stubs in font_test_specs to avoid having to re-name all of the
later tests.
@anntzer
Copy link
Copy Markdown
Contributor Author

anntzer commented Feb 9, 2020

rebased

@tacaswell tacaswell merged commit d311b9b into matplotlib:master Feb 10, 2020
@anntzer anntzer deleted the unmathcircled branch February 10, 2020 05:04
@anntzer anntzer mentioned this pull request Feb 12, 2020
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tacaswell tacaswell tacaswell approved these changes

@dstansby dstansby Awaiting requested review from dstansby

@timhoffm timhoffm Awaiting requested review from timhoffm

Assignees

No one assigned

Labels

API: changes Maintenance topic: text/mathtext

Projects

None yet

Milestone

v3.3.0

Development

Successfully merging this pull request may close these issues.

4 participants

@anntzer @tacaswell @timhoffm @dstansby