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
BUG : fix eps corruption when using clipping #3526
Conversation
_get_clip_path was returning '<built-in function id>' instead of the ps-function name like it should have been. This was introduced in 9b9c0c6 in PR matplotlib#2927. Closes matplotlib#3523
660dc47
to
3424506
Compare
If you don't mind, I would like to add @bryan-woods to this discussion. I have been meaning to show him how a PR works, and this is a good way to see how it works. |
@@ -568,7 +568,7 @@ def _get_clip_path(self, clippath, clippath_transform): | |||
ps_cmd.extend(['clip', 'newpath', '} bind def\n']) | |||
self._pswriter.write('\n'.join(ps_cmd)) | |||
self._clip_paths[key] = pid | |||
return id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't the PEP8 tool notice this? I know pylint would have warned about namespace collisions. Then again, I guess it isn't really a collision as much as a typo.
Also, why didn't any of our tests see this? What was it about basemap that triggered this? |
Looking back at that referenced PR, I see that the original code did use "id" and it was changed to "pid" everywhere except in the return. Lovely... |
BUG : fix eps corruption when using clipping
Tests didn't catch it because we don't test the ps backend and apparently no one ever tried to open the file that was generating the issue in the other PR. pep8 does not do any linting, just white space. |
_get_clip_path was returning '' instead
of the ps-function name like it should have been. This was introduced
in 9b9c0c6 in PR #2927.
Closes #3523