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

inset_locator.mark_inset() misplaces box connectors #17711

Closed
joreiff opened this issue Jun 22, 2020 · 8 comments · Fixed by #21519
Closed

inset_locator.mark_inset() misplaces box connectors #17711

joreiff opened this issue Jun 22, 2020 · 8 comments · Fixed by #21519

Comments

@joreiff
Copy link

joreiff commented Jun 22, 2020

Bug report

Bug summary

inset_locator.mark_inset() sometimes misplaces the box connector lines if x/y limits on the parent Axes have not been explicitly set. Updating the plot (e.g. by saving the figure) seems to fix it.

Code for reproduction

import matplotlib.pyplot as plt
import mpl_toolkits.axes_grid1.inset_locator as miloc

fig, (inset, full) = plt.subplots(ncols=2)

# swapping the axes makes the example work
# full, inset = inset, full

full.plot([-1, 0], [-1, 0])
inset.set_xlim(-0.6, -0.4)
inset.set_ylim(-0.6, -0.4)

# workaround
# full.set_xlim(-1.0, 0.0)
# full.set_ylim(-1.0, 0.0)

miloc.mark_inset(full, inset, 1, 4)
fig.savefig('Bug.png')
# saving the figure a second time produces the expected result
fig.savefig('NoBug.png')

Actual outcome

Bug

Expected outcome

NoBug

This issue was not present in version 3.0.2.

Matplotlib version

  • Operating system: Fedora and Debian
  • Matplotlib version: 3.2.1 and 3.2.2
  • Matplotlib backend (print(matplotlib.get_backend())): Qt5Agg and agg
  • Python version: 3.8
  • Jupyter version (if applicable): N/A
  • Other libraries: N/A

Installed via pip on Debian and both pip and dnf on Fedora.

@jklymak
Copy link
Member

jklymak commented Jun 22, 2020

I don't know if you need some feature of the axes_grid1 implementation, but the following seems to work fine:

import matplotlib.pyplot as plt

fig, (inset, full) = plt.subplots(ncols=2)

full.plot([-1, 0], [-1, 0])
inset.set_xlim(-0.6, -0.4)
inset.set_ylim(-0.6, -0.4)

full.indicate_inset_zoom(inset)
plt.show()

inset

@jklymak
Copy link
Member

jklymak commented Jun 22, 2020

For me, the bug with axis_grid1 bisects to 160de56, #13593 , which probably makes sense, but I've not thought about it too hard yet. @anntzer ?

@anntzer
Copy link
Contributor

anntzer commented Jun 22, 2020

Yes, I understand the bug, but I think it's rather tricky to fix unless we introduce a full loop in Figure.draw where we go through all axes and autoscale them all (I guess perhaps at the same time as running the locators to position them...) before actually drawing them...

@jklymak
Copy link
Member

jklymak commented Jun 22, 2020

I'd see if the OP really needs this to work in axes_grid1 or if they will be happy with the more "modern" version. Even when it worked the result is not particularly nice in axes_grid1....

@joreiff
Copy link
Author

joreiff commented Jun 22, 2020

Thanks for the fast reply! I did discover that calling full.get_xlim() before saving/showing the figure fixes the problem, so I have a workaround for now.

The reason I did not use indicate_inset_zoom is that

  1. I was working on existing code that preceded this function,
  2. I have more code to make mark_inset pretty and
  3. indicate_inset_zoom is marked as experimental in the documentation.

If you would consider indicate_inset_zoom stable enough, however, I can switch to it in the future.

@tacaswell tacaswell added this to the v3.4.0 milestone Jun 22, 2020
@jklymak
Copy link
Member

jklymak commented Jun 23, 2020

@joreiff, I think the main library' version is probably stable. We've not received too many complaints about it, which either means it works fine, or no one is using it 😉

@joreiff
Copy link
Author

joreiff commented Jun 24, 2020

@jklymak In this case, I will be happy using the new API in the future. If fixing this bug is tricky and indicate_inset_zoom is functionally equivalent, it may be a good idea to deprecate mark_inset at some point. Feel free to close this issue if you like.

@jklymak
Copy link
Member

jklymak commented Jun 24, 2020

No, it should definitely stay open. I doubt axes_grid1 will ever be completely deprecated, and someone may want to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants