Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Update MovieWriter dpi default #8063
Conversation
| @@ -165,7 +165,7 @@ def setup(self, fig, outfile, dpi): | ||
| The filename of the resulting movie file | ||
| dpi: int | ||
| The DPI (or resolution) for the file. This controls the size | ||
| - in pixels of the resulting movie file. | ||
| + in pixels of the resulting movie file. Default is figure.dpi. |
|
@tacaswell I just updated the docstrings. I just noticed that in Animate.save() has dpi=None default to rcParam['savefig.dpi'], and only if this is 'figure' it is set to figure.dpi. Should I use that behavior instead of setting None to figure.dpi? |
|
On one hand what you have here is simpler and does not depend on global state, on the other hand reaching out to I am leaning towards not defaulting to |
tacaswell
added this to the
2.1 (next point release)
milestone
Feb 11, 2017
tacaswell
changed the title from
Update MovieWriter dpi default to [MRG+1] Update MovieWriter dpi default
Feb 11, 2017
| The DPI (or resolution) for the file. This controls the size | ||
| - in pixels of the resulting movie file. | ||
| + in pixels of the resulting movie file. Default is figure.dpi. |
dopplershift
Feb 15, 2017
Contributor
Should figure.dpi be fig.dpi since fig is the name of the argument?
| The DPI (or resolution) for the file. This controls the size | ||
| - in pixels of the resulting movie file. | ||
| + in pixels of the resulting movie file. Default is figure.dpi. |
| The dpi of the output file. This, with the figure size, | ||
| controls the size in pixels of the resulting movie file. | ||
| + Default is figure.dpi. |
dopplershift
added the
Animation
label
Feb 15, 2017
|
@tacaswell @dopplershift Thanks, I updated in (c2aac89). Let me know if should rebase this on master/squash some of these commits to something cleaner. |
|
If it's not a lot of effort, I'd prefer to see this squashed down a bit. |
|
@dopplershift I squashed all the lints/nits into the original commit so it looks like it never happened. I also rebased on the current master because I was a few commits behind. |
NelleV
changed the title from
[MRG+1] Update MovieWriter dpi default to [MRG+2] Update MovieWriter dpi default
Feb 16, 2017
dopplershift
merged commit f697e8c
into matplotlib:master
Feb 16, 2017
dopplershift
changed the title from
[MRG+2] Update MovieWriter dpi default to Update MovieWriter dpi default
Feb 16, 2017
|
Thanks @heath730 ! |
heath730 commentedFeb 11, 2017
Adds default
dpi=Noneargument to *MovieWriter classes, and usesfigure.dpias the default if dpi is None.This is related to issue #7616. @tacaswell please advise if this approach is what you had in mind for resolving the issue.
I'm a new contributor and I wasn't exactly sure the best way to unit test a change like this. However, I created a test that calls setup with dpi as the default, and makes sure that dpi is set to the
figure.dpi. I did not add a change log entry because it is such a small change, please let me know if I should.