Jpeg quality 95 by default with rendering with PIL

[dhyams]

This is just my opinion, but when rendering a plot to JPEG, the default quality should be a bit higher. Plots are usually (admittedly not always) full of starkly defined edges, that don't look very good once the jpeg format gets finished artifacting around all of them.

While it's true that a user shouldn't write a jpeg of something that doesn't consist of smoothly varying colors, if they do, let's try to make it look as good as is reasonable.

I've attached two images, one with the 75 quality and one with 95. The 75 quality one was 18K in size, and the 95 quality was 30K in size.

[mdboom]

👍 from me. I don't think disk space or bandwidth are at such a premium that this doesn't make sense as a default.

[pelson]

Why not have a .matplotlibrc parameter for this, that way users can set it to whatever they like? I'd be happy enough for 95 to be the default.

[mdboom]

@pelson: Good suggestion.

[dhyams]

On the above commit; I'm not sure what the best practices are when dealing with rcParams (as far as whether to assume the entry is always there, and provide an initialization for it (savefig.jpeg_quality), or to use rcParams.get() as I have in the commit.

Also should the parameter be named savefig.pil_jpeg_quality or something like that, to indicate that it's only used for PIL?

[mdboom]

The rcParam should be defined in rcsetup.py and described in matplotlibrc.template -- then you don't need to worry about it being defined or not -- it will always have a default value even if the user doesn't specify it.

As for the name -- it would be nice to fix up all of the backends that write jpegs to support this (it looks doable, just a matter of working through all the various APIs), so I'd be in favor of calling it savefig.jpeg_quality and then creating issues for all of the backends with JPEG support to start supporting this parameter.

[dhyams]

On the above commits:

1. I was unable to test gdk and gtk on my current setup; so that's still pending.
2. quality parameter not supported in the mac backend. I think it shouldn't be too hard, but I'm unable to test it at the present time, and it takes some mucking around with some objective C code to make it work.
3. otherwise, works fine on the wx and wxagg backends (which are really the same, in this branch anyway; see strange output from wx and wxagg when trying to render to JPEG or TIFF #1770 for more info on this)

[pelson]

PEP8: Please use just one newline between blocks of code.

[dhyams]

do you mean between lines 477 and 478?

[pelson]

@dhyams - looks like you've done most of the hard work. The PR wont merge automatically, so would you mind re-basing. I'd also be interested to hear what pieces of work you think are critical to be done before this PR should be merged.

Thanks,

[pelson]

Bump. I'm keen to get this into v1.3 (~ a months time)

[dhyams]

Sorry; I have been a little bogged down lately and let this one slip. I'll make the requested changes as soon as I can, and rebase.

[dhyams]

Re: pelson on what else needs to be done before this PR is merged: maybe can someone test on gtk and gdk? My current setup precludes the use of these two backends.

[dhyams]

git clone git@github.com:dhyams/matplotlib.git
git remote add original git://github.com/matplotlib/matplotlib.git
git checkout jpeg_quality_95_by_default
git rebase origin/master
conflict happens, so I edit to resolve the conflict
git add matplotlibrc.template
git rebase --continue

At this point, everything looks OK.

git push

To git@github.com:dhyams/matplotlib.git
! [rejected] jpeg_quality_95_by_default -> jpeg_quality_95_by_default (non-fast-forward)
error: failed to push some refs to 'git@github.com:dhyams/matplotlib.git'
To prevent you from losing history, non-fast-forward updates were rejected
Merge the remote changes (e.g. 'git pull') before pushing again. See the
'Note about fast-forwards' section of 'git push --help' for details.

Is it correct to do a

git push -f

[mdboom]

I usually run the test suite again once the conflicts have been resolved to make sure, but after that, yes, git push -f should be fine.

[WeatherGod]

What probably happened is that you just did a straight out rebase of a
branch that used to be based on your personal copy of master, and is now on
the upstream copy of master. What I like to do is to keep my local
repository up to date by doing "git fetch --all" and updating the relevant
tracking branches (e.g., my copies of master and v1.2.x and so on) by
rebasing them with the upstream counterparts. I push those up to my github
repo.

Then, when it is time to rebase a feature branch, I rebase against my local
tracking branches, which i know to be fresh. Deal with conflicts, and then
push that up to my repo. Not foolproof, but it has kept my repos clean.

Cheers!
Ben Root

On Fri, Apr 19, 2013 at 6:50 PM, Michael Droettboom <
notifications@github.com> wrote:

I usually run the test suite again once the conflicts have been resolved
to make sure, but after that, yes, git push -f should be fine.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1771#issuecomment-16693714
.

[dhyams]

I just pushed the rebasing on this one.

[dhyams]

Bump, so that the rebasing doesn't get out of date. Is this one OK to merge?

[pelson]

Thanks for merging @mdboom - I think the code changes were ready to go & @dhyams was right to encourage a merge before the PR went stale.

@dhyams: Given there is a change to the default JPEG quality, and there is a new rcParam, would you mind creating a new PR with the necessary changes to the doc/api/whats_new.rst (for the fact that the value has changed from 75 to 95) and users/whats_new.rst (for the fact that there is a new rcParam for JPEG quality).

While you're at it, this PR has introduced a couple of double newlines (one in lib/matplotlib/backends/backend_gtk.py and one in lib/matplotlib/backends/backend_wx.py). Whilst these are cosmetic, we are trying to standardise towards PEP8 (specifically http://www.python.org/dev/peps/pep-0008/#blank-lines); would you mind tidying these up? (this is the epitome of nitpicking, for which I apologise 😄)

[dhyams]

Ok this has been done here:

#1940

[pelson]

Brilliant. Thanks @dhyams!