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

PEP8 fixes on the figure module #1716

Merged
merged 2 commits into from Feb 18, 2013
Merged

PEP8 fixes on the figure module #1716

merged 2 commits into from Feb 18, 2013

Conversation

NelleV
Copy link
Member

@NelleV NelleV commented Jan 29, 2013

Here is a small patch for pep8 fixes on the figure module.

Thanks,
N

@@ -346,9 +349,9 @@ def show(self, warn=True):
try:
manager = getattr(self.canvas, 'manager')
except AttributeError as err:
raise AttributeError("%s\n Figure.show works only "
Copy link
Member

Choose a reason for hiding this comment

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

This \n nicely separates the error message from the rest of the message:

AttributeError: 'FigureCanvasPdf' object has no attribute 'manager'
    Figure.show works only for figures managed by pyplot,
    normally created by pyplot.figure().

Is it bad practice to leave new line characters in the middle of strings like this?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to have the \n left in -- it certainly improves the output. Maybe the middle ground is to also put the string breaks in the same place, eg:

raise AttributeError(
    "%s\n"
    "Figure.show works only for figures managed by pyplot,\n"
    "normally created by pyplot.figure()." % err)

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added it back. In general, doing formatting of error messages "manually" isn't a good thing. Here, I agree it is better to separate the original error message to the more understandable matplotlib one.

efiring added a commit that referenced this pull request Feb 18, 2013
PEP8 fixes on the figure module
@efiring efiring merged commit dfd0345 into matplotlib:master Feb 18, 2013
@NelleV NelleV deleted the pep8_figure branch September 6, 2016 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants