-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Parallel movie writing routines. #4509
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
Conversation
Parallel movie recording routines for Python's matplotlib (see #2820 for the motivation and the discussion on design/implementation).
Example usage for mpl_parsave
# processes. | ||
# ========================================================================# | ||
class Parsave: | ||
def __init__(self): |
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.
4 spaces please to match the rest of the library.
attn @WeatherGod @dopplershift |
Changed tabs from two to four spaces.
Hmm... can't see why the build failed. |
There are intermittent failures related to latex, restarted it. |
Thank you for your contribution, @william-yessen, this seems like it'll be a nice feature to add to matplotlib. I happened to browse github casually today, so I don't have detailed feedback on the implementation, my review is mostly style related. If you are able, I encourage you to take some time to meet our coding style guide. In particular:
Let us know if you have any questions (or if it sounds like too much work). |
Thanks for the tips! No, it's not a problem, I can do all of that. Been a bit busy these days, maybe next week. |
Hmm... any idea why the checks failed? |
There are intermittent failures in the test suite. Closed/re-opened PR to re-trigger the test suite on top of current master. |
Thanks, Thomas! |
Do we want to merge this as-is, or does this make sense as a separate, but affiliated, project? |
@mdboom Currently it resides in the toolkits. Would that be an appropriate place for it? |
The example should move into |
|
||
class Parsave(object): | ||
""" | ||
Parsave: class containing static methods to fascilitate parallel recording. |
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.
The rest of the code-base is 4 space indent
OK, all of the comments make sense. I'll work on it a bit later. |
This should also get an entry in https://github.com/matplotlib/matplotlib/tree/master/doc/users/whats_new Sorry to bombard you with mostly trivial comments, but keeping the style across the code base consistent does make maintenance easier in the long run. |
How would this work together with #5454? On Tue, Nov 17, 2015 at 2:13 PM, Thomas A Caswell notifications@github.com
|
It's not inappropriate. But we are trying to move away from "incorporating" new toolkits and just releasing tools as separate projects, on their own release schedule etc. I don't have strong opinions about this one -- it's pretty small and self-contained. I could be persuaded either way. |
Is there a way to turn this into something that looks like the 'normal' writer object? |
@tacaswell Yes, I think so. I toyed with the idea a while back, but figured at the end that the way it is currently implemented is simpler and easier to maintain. If, however, there is sufficient reason for it to be a 'normal' writer object, I can work on that. |
The argument in favor of making it all the same api is that now users can just change a class to flip between parallel vs not moving writing which makes their code simpler and easier to maintain. |
Closing because of lack of comments for almost 2 years and the OP has deleted their account. |
Parallel movie recording routines for Python's matplotlib (see #2820 for the motivation and the discussion on design/implementation).