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

ENH: raise a more informative error #1533

Merged
merged 3 commits into from Nov 30, 2012
Merged

Conversation

jakevdp
Copy link
Contributor

@jakevdp jakevdp commented Nov 25, 2012

Raise a more informative error when save is called and no animation writers are available on the system. Previously this situation would lead to an IndexError at the line writer = writers.list()[0]

@pelson
Copy link
Member

pelson commented Nov 26, 2012

Thanks @jakevdp. My biggest problem with this change is that I now have to use the writers registry (which I don't use). I think a better approach would be to add a new method on to the writers registry which can return the "first" (in speech marks because we have inconsistent ordering due to use of a dictionary) available writer, or raise an exception (not an IndexError as it used to).

Other than that, I'm in favour of seeing this exception improved.

Thanks,

@jakevdp
Copy link
Contributor Author

jakevdp commented Nov 26, 2012

Ah, I didn't think of the possibility of people passing anything but a string to the writer argument... yes, that should be improved.

@jakevdp
Copy link
Contributor Author

jakevdp commented Nov 27, 2012

This update should fix the issue.


try:
writer = writers.list()[0]
except:
Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry for being so picky, but ideally we should avoid bare try..except blocks. I think your trying to improve the IndexError which ensues when there are no writers? In which case, would you mind updating this to a try ... except IndexError:.

After that, its good to merge - honest! 😄

dmcdougall added a commit that referenced this pull request Nov 30, 2012
ENH: raise a more informative error
@dmcdougall dmcdougall merged commit dd92a43 into matplotlib:master Nov 30, 2012
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

3 participants