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

Factor out common code between pdf and ps backends. #9867

Merged
merged 1 commit into from
Jan 8, 2019

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Nov 27, 2017

(The AFM cache only needs to cache on filename, not on font properties,
because the fontprop->filename conversion is already cached by
font_manager.)

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 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 force-pushed the pdfps-common branch 3 times, most recently from fc50040 to 2ddaed9 Compare November 27, 2017 15:44
@tacaswell tacaswell added this to the v2.2 milestone Nov 27, 2017
@jklymak
Copy link
Member

jklymak commented Nov 27, 2017

Something is wrong w/ py 2.7 figures.

@anntzer
Copy link
Contributor Author

anntzer commented Nov 27, 2017

forgot to future-import division :p

@jklymak
Copy link
Member

jklymak commented Nov 28, 2017

appveyor seems to have picked up a bug. Both your recent PRs have it....

@anntzer
Copy link
Contributor Author

anntzer commented Nov 28, 2017

Seems unrelated, tbh...

@jklymak
Copy link
Member

jklymak commented Nov 28, 2017

No I agree, it has nothing to do w/ your PRs, but I suspect a bug was recently introduced into Master

@QuLogic
Copy link
Member

QuLogic commented Nov 28, 2017

The last build to pass had pytest 3.2.5; this one has 3.3.0. There was a similar bug in 3.2.1: pytest-dev/pytest#2644 but maybe there was a regression.

@QuLogic
Copy link
Member

QuLogic commented Nov 28, 2017

New report is at pytest-dev/pytest#2957.

@anntzer anntzer force-pushed the pdfps-common branch 2 times, most recently from 04546a2 to 117acd0 Compare February 17, 2018 23:05
@anntzer anntzer modified the milestones: needs sorting, v3.0 Feb 26, 2018
@jklymak
Copy link
Member

jklymak commented Jan 4, 2019

This seems like a useful re-factor. It could use a second review... ping @tacaswell @jkseppan

@anntzer
Copy link
Contributor Author

anntzer commented Jan 4, 2019

rebased

Additionally merged the implementations of get_canvas_width_height, with the following addition to the commit message:

get_canvas_width_height can be merged because FigureCanvasPdf always
makes sure to call newPage(width, height) and create a new RendererPdf
with the same width and height, and it's likely other things break if
the PdfFile and the Renderer's size don't match anyways.

@anntzer anntzer force-pushed the pdfps-common branch 3 times, most recently from 3460602 to 899939b Compare January 4, 2019 18:28
(The AFM cache only needs to cache on filename, not on font properties,
because the fontprop->filename conversion is already cached by
font_manager.

get_canvas_width_height can be merged because FigureCanvasPdf always
makes sure to call newPage(width, height) and create a new RendererPdf
with the same width and height, and it's likely other things break if
the PdfFile and the Renderer's size don't match anyways.
@anntzer
Copy link
Contributor Author

anntzer commented Jan 8, 2019

@jklymak @timhoffm either of you want to click on the green button? :)

@jklymak jklymak merged commit 2b86111 into matplotlib:master Jan 8, 2019
@anntzer anntzer deleted the pdfps-common branch January 8, 2019 15:10
@anntzer
Copy link
Contributor Author

anntzer commented Jan 8, 2019

thanks

@jklymak
Copy link
Member

jklymak commented Jan 8, 2019

Thank you!

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.

5 participants