Skip to content

Pdf backend - #8644

Closed
ghost wants to merge 2 commits into
masterfrom
unknown repository
Closed

ghost wants to merge 2 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented May 19, 2017 •

Copy link
Copy Markdown

PR Summary

Small pull request to address #8142. The issue seems to propagate from a feature enhancement #5596 where Line2D.get_markeredgecolor() returns None due to the change-set. The fix simply check if gc._rgb == None in backendpdf (caused the said change-set), if so, set gc._rgb to be the value it would have taken if the change-set was not there (pre #5596 behavior). Hence, this can probably be viewed as an addition to #5596 where behaviour of backendpdf remains the same .

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/whats_new.rst if major new feature
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way


orig_alphas = getattr(gc, '_effective_alphas', (1.0, 1.0))
if gc.get_rgb() is None:
gc.set_foreground([0.0, 0.0, 0.0, 1.0])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why black? This should probably have 0 alpha?

@ghost ghost May 19, 2017 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm not entirely sure what the alpha value has to do. I simply revert the unwanted behavior of #5596 i.e. in the function Line2D.draw(renderer), with the above setting, prior to the the change-set, at line 830, edgecolor (returned by Line2D.get_markeredgecolor())would be 'k', and after the call gc.set_foreground(edgecolor) on line 835, gc._rgb takes the value that I set. Will put a test case tomorrow to check to be sure that I did not miss anything.

@tacaswell

Copy link
Copy Markdown
Member

Can you add a test? How to the png and pdf look with this change?

@tacaswell tacaswell added this to the 2.1 (next point release) milestone May 19, 2017
@ghost

ghost commented May 21, 2017

Copy link
Copy Markdown
Author

Note: I'm having a bit of difficulty comparing PDF image to png image, so I might modify your compare_image function and include it in the PR to make this process easier later too.

@ghost ghost closed this May 21, 2017
@tacaswell

Copy link
Copy Markdown
Member

Sorry, I meant to just look at them, not to progamtically compare them!

This pull request was closed.
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.

2 participants