Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Break reference cycle Line2D <-> Line2D._lineFunc.#6821
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Looks good to me. Why didn't Travis run? |
Not sure why... it built on my side:https://travis-ci.org/anntzer/matplotlib/builds/146967616 (docsnever build for me, btw... suggestions welcome). |
| @@ -236,7 +236,7 @@ def test_grid(): | |||
| fig = manager.canvas.figure | |||
| ax = fig.add_subplot(1, 1, 1) | |||
| ax.grid() | |||
| # Drawing the gridtriggers instance methods to be attached | |||
| # Drawing the gridused to trigger instance methods to be attached | |||
| # to the Line2D object (_lineFunc). | |||
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.
_lineFunc
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.
?
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.
Probably best to delete the whole comment; it is no longer relevant, right?
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.
Should we just get rid of the whole test? Given that it is exactly there to test the workaround that this PR removes.
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.
Good point--I didn't notice that this was in a test. Yes, I think removing the test makes perfect sense.
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.
done.
jenshnielsen commentedJul 24, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The docs build fails for you on travis because the install of basemap causes matplotlib to be downgraded to 1.5.1. This is most likely because your fork does not have all the tags which confuses the versioneer version script. I suggest pushing the tags to your branch. |
I opened and closed the pr to re trigger travis |
Thanks for the help re: docs. |
Upon drawing, Line2D objects would store a reference to one of their ownbound methods as their `_lineFunc` argument. This would lead to thembeing gc'ed not when going out of scope, but only when the "true" gckicks in; additionally this led to some pickle-related bugs (matplotlib#3627).One can easily sidestep this problem by not storing this bound method.To check the behavior, try (py3.4+ only):```import gcimport weakreffrom matplotlib import pyplot as pltdef f(): fig, ax = plt.subplots() img = ax.imshow([[0, 1], [2, 3]]) weakref.finalize(img, print, "gc'ing image") l, = plt.plot([0, 1]) weakref.finalize(l, print, "gc'ing line") fig.canvas.draw() img.remove() l.remove()f()print("we have left the function")gc.collect()print("and cleaned up our mess")```Before the patch, the AxesImage is gc'ed when the function exits but theLine2D only upon explicit garbage collection. After the patch, both arecollected immediately.
| @@ -1250,9 +1244,6 @@ def set_dashes(self, seq): | |||
| else: | |||
| self.set_linestyle((0,seq)) | |||
| def_draw_lines(self,renderer,gc,path,trans): | |||
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_drawStyles_l dictionary still references_draw_lines, and so I would imagine that by removing this method would cause problems (but I am not sure I am clear how it would have worked properly in the first place?).
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 entiredrawStyles dict is now unused, and only left for backcompatibility purposes (mostly to have alist of drawstyles available asmatplotlib.lines.drawStyles). With this change none of the values actually refer to a method name anymore. I killed the other entries in#6497 for another purpose.
(That's also the reason why this can't go in 2.x: this depends on#6497 (and its followup,#6645)).
btw, in case it isn't clear, this cannot be backported to v2.x branch as-is because there are more usages of |
👍 from me. Thanks for cleaning out our cruft. |
Upon drawing, Line2D objects would store a reference to one of their own
bound methods as their
_lineFuncargument. This would lead to thembeing gc'ed not when going out of scope, but only when the "true" gc
kicks in; additionally this led to some pickle-related bugs (#3627).
One can easily sidestep this problem by not storing this bound method.
To check the behavior, try (py3.4+ only):
Before the patch, the AxesImage is gc'ed when the function exits but the
Line2D only upon explicit garbage collection. After the patch, both are
collected immediately.