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

Draggable legend changes position when saving figure with DPI value different from display #10458

Closed
AgilentGCMS opened this issue Feb 14, 2018 · 17 comments

Comments

Projects
None yet
4 participants
@AgilentGCMS
Copy link

commented Feb 14, 2018

Bug report

Bug summary

If I add a draggable legend to a plot, then move it to a desired position by dragging it, then save the plot to a PDF by using savefig, the legend position on the PDF is not the same as the legend position I dragged it to on the screen.

Code for reproduction

from matplotlib import pyplot as plt
import numpy as np

x = np.linspace(-2.*np.pi, 2.*np.pi, 1000)
y = np.sin(x)
plt.plot(x, y, '-', lw=2, label='Sine curve')
leg = plt.legend(loc='best', fontsize=20)
leg.draggable(True)
plt.show()

After running this code, drag the legend manually to the lower left of the plot, then issue the command

plt.savefig('sine_curve.pdf')

Actual outcome

On the screen, after dragging the legend the plot looks like plot_on_screen.png. However, the saved PDF looks like sine_curve.pdf. Notice how the legend is obviously at a different location than where I placed it by dragging.
plot_on_screen
sine_curve.pdf

Expected outcome

The legend should stay in the same place in the PDF as on the screen. This used to be the case with matplotlib 1.5.3.

Matplotlib version

  • Operating system: Linux x64
  • Matplotlib version: 2.1.2
  • Matplotlib backend: Qt4Agg
  • Python version: 2.7.6
  • Jupyter version (if applicable): N/A
  • Other libraries:

I installed matplotlib from source.

@tacaswell tacaswell added this to the v2.2.1 milestone Feb 14, 2018

@tacaswell

This comment has been minimized.

Copy link
Member

commented Feb 14, 2018

Pretty sure this is because the draggable legend saves it's position in screen units and does not account for the changes in dpi between the screen and pdf.

If you set the figure dpi to 72 does it work as expected?

@AgilentGCMS

This comment has been minimized.

Copy link
Author

commented Feb 14, 2018

Just tried plt.savefig('sine_curve.pdf', dpi=72), and the result is the same (see attached).
sine_curve.pdf

I thought the dpi did not need to be specified for PDF output? I've only ever specified it for raster output.

@tacaswell

This comment has been minimized.

Copy link
Member

commented Feb 14, 2018

I mean adjust the dpi of the figure on the screen. The default dpi is now 100, it used to default to 80. The dpi in pdf is always 72. I suspect it always moved a bit, but 80 an 72 are close enough that it was subtle, with the screen figure at 100 it is obvious.

x = np.linspace(-2.*np.pi, 2.*np.pi, 1000)
y = np.sin(x)
fig, ax = plt.subplots(dpi=72)
ax.plot(x, y, '-', lw=2, label='Sine curve')
leg = ax.legend(loc='best', fontsize=20)
leg.draggable(True)
plt.show()
@AgilentGCMS

This comment has been minimized.

Copy link
Author

commented Feb 14, 2018

Indeed, specifying the screen dpi=72 fixed it. Is that a permanent fix? Or is there a way for savefig to convert properly between different dpis?

More importantly, is there a way to globally set the screen dpi to 72, say in the rc file? I use draggable legends a lot, and would rather not specify it for each figure.

@tacaswell

This comment has been minimized.

Copy link
Member

commented Feb 14, 2018

I would call it a workaround, not really a fix ;)

The bug is in the draggable legend not in savefig. It stores "the user put my XX pixels from the edge" which is a different real distance in the figure depending on the DPI. The legend should either be changed to use a relative unit ("the user put me (0.1 * the axes width) from the edge" or be able to do the DPI correction at draw time.

Yes figure.dpi (see https://matplotlib.org/users/customizing.html#a-sample-matplotlibrc-file).

@tacaswell

This comment has been minimized.

Copy link
Member

commented Feb 14, 2018

Labeled as medium because iirc the draggable code is a bit opaque. I suspect that once the correct place to fix this is found it will be straight foreword, but finding that place may be tricky (and I would be very happy to be proved wrong).

@AgilentGCMS

This comment has been minimized.

Copy link
Author

commented Feb 14, 2018

OK, the workaround of setting figure.dpi to 72 is working for me now, thanks!

@afvincent

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2018

FWIW, here is a manual testing script that shows the issue between the DPI value used for the display, and the DPI value used when saving the figure.

import numpy as np
import matplotlib.pyplot as plt

x = np.linspace(-2.*np.pi, 2.*np.pi, 1000)
y = np.sin(x)

fig, ax = plt.subplots(dpi=100, num='test_draggable_legend')
ax.plot(x, y, '-', lw=2, label='Sine curve')
ax.set_title("Display @ {} dpi".format(fig.get_dpi()))

leg = ax.legend(loc='best', fontsize=20)
leg.draggable(True)

plt.show()  # then move a bit the legend and close the figure

for ext, dpi in [('.png', 72), ('.png', 100), ('.png', 144), ('.pdf', None)]:
    suffix = '_figsave_{}_dpi'.format(dpi if dpi is not None else 'None')
    fig.savefig(fig.get_label() + suffix + ext, dpi=dpi)

produces

  • figsave @ 72 dpi (not OK with the display): test_draggable_legend_figsave_72_dpi
  • figsave @ 100 dpi (OK with the display): test_draggable_legend_figsave_100_dpi
  • figsave @ 144 dpi (not OK with the display): test_draggable_legend_figsave_144_dpi

NB: I am going to edit the thread title as the issue does not seem specific to PDF.

Edit: the plots were produced with the master branch on a Linux system (Fedora 27) and with Python 3.6 from conda.

@afvincent afvincent changed the title Draggable legend changes position when saving as PDF Draggable legend changes position when saving figure with DPI value different from display Feb 14, 2018

@AgilentGCMS

This comment has been minimized.

Copy link
Author

commented Feb 14, 2018

OK, in that case this is definitely a new bug. I've been using draggable legends for several years, and I often save to PNG files at different dpi's for different purposes (poster, presentation, printing, etc.) Until I updated to matplotlib 2.1.2, those PNG files looked fine at whatever dpi I saved them.

@afvincent

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2018

Ok then from git bisect, I get:

199a87a71a22dffc495bc84a97893ac3480e7b43 is the first bad commit
commit 199a87a71a22dffc495bc84a97893ac3480e7b43
Author: Antony Lee <anntzer.lee@gmail.com>
Date:   Sun Jan 8 15:26:35 2017 -0800

    Don't check `iterable()` before `len()`.
    
    ... because the former does not imply the latter anyways, e.g.
    generators are iterables but unsized.
    
    Now `plot(zip([1, 2], [3, 4]))` (py3) raises `RuntimeError: matplotlib
    does not support generators` (from `cbook.safe_first_element`) which is
    probably the intended exception, rather than `TypeError: object of type
    'zip' has no len()`.  Perhaps this exception should be changed to a
    `TypeError`, by the way...

which seems plausible when looking at the effective commit (not simply the commit message ^^). Pinging @anntzer who is the author of the aforementioned commit.

@afvincent

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2018

Well, I do not really understand why, but the following patch seems to make the behavior of a draggable legend as consistent as before :/. (I simply reverted the one line that I was not sure to understand in @anntzer 's commit...)

From 8b73cb2270192cd7ed5b2a35b14e9de2aca43fd7 Mon Sep 17 00:00:00 2001
From: "Adrien F. Vincent" <vincent.adrien@gmail.com>
Date: Wed, 14 Feb 2018 17:06:02 -0800
Subject: [PATCH 2/2] fix draggable legend offset changes with dpi value

---
 lib/matplotlib/legend.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/matplotlib/legend.py b/lib/matplotlib/legend.py
index 08b5c45de..7ab78c76a 100644
--- a/lib/matplotlib/legend.py
+++ b/lib/matplotlib/legend.py
@@ -731,6 +731,7 @@ class Legend(Artist):
         # value of the find_offset.
         self._loc_real = loc
         self.stale = True
+        self._legend_box.set_offset(self._findoffset)
 
     def _get_loc(self):
         return self._loc_real
@@ -1002,7 +1003,6 @@ class Legend(Artist):
                                    children=[self._legend_title_box,
                                              self._legend_handle_box])
         self._legend_box.set_figure(self.figure)
-        self._legend_box.set_offset(self._findoffset)
         self.texts = text_list
         self.legendHandles = handle_list
 
-- 
2.14.3

Edit: English

@afvincent

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2018

I think that I know undertand why the patch above is working, but somebody else might want to confirm if I am right or not.

When self._legend_box.set_offset(self._findoffset) is only applied in Legend._init_legend_box, then in the case of a (derived) DraggableLegend instance, the corresponding attribute in the parent OffsetBox is overwritten after (each) “pick event” due to self.offsetbox.set_offset(offset) in DraggableOffsetBox.save_offset. This is because OffsetBox works in display coordinates (cf. its docstring), while we would like DraggableLegend to work in its parents relative coordinates, would we not?

On the contrary, applying self._legend_box.set_offset(self._findoffset) each time that Legend._set_loc is called (i.e. the “original” approach) “over-overwrites” (overwriception!) the relevant attribute after each “pick event”, cancelling the change. Not sure that it is the most efficient way to get the result that we want, but it seems to be working...

@anntzer

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2018

I think the patch you are proposing is correct, and should probably be considered a bugfix and go to 2.2.0. Sorry for the bug introduced.

@anntzer anntzer modified the milestones: v2.2.1, v2.2.0 Feb 15, 2018

@afvincent

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2018

Thanks @anntzer , I am going to open a PR. To be fair, the bug was easy to miss as we do not really test interactive stuff in the test suite, do we?

@afvincent afvincent referenced this issue Feb 15, 2018

Merged

FIX: DPI inconsistency of draggable legend #10469

0 of 2 tasks complete
@afvincent

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2018

@tacaswell For the record, the proposed fix is indeed quite straightforeward, as you were suggesting it would be ;).

@afvincent

This comment has been minimized.

Copy link
Contributor

commented Feb 17, 2018

Fixed by 8b73cb2 in #10469 . Closing.

@afvincent afvincent closed this Feb 17, 2018

@tacaswell

This comment has been minimized.

Copy link
Member

commented Feb 18, 2018

👍 thanks @afvincent !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.