Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Move required_interactive_framework to canvas class.#14521
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
In mpl 3.0, the required_interactive_framework attribute was added tobackend modules. However, it is sometimes required to access it from acanvas *instance* (e.g. in _fix_ipython_backend2gui), but the pattern`sys.modules[cls.__module__].required_interactive_framework` is brittle(e.g. it is broken by third-party subclasses).Instead it makes more sense to put the attribute on the canvas class;one can easily and robustly go from the backend module to the canvasclass by accessing the FigureCanvas attribute.
Should we leave it in both places? That doesn't break any API but it does risk them getting out of sync. Other than than 👍. Unfortunately, module level |
Assuming we want to make it easier to define 3rd-party backends, requiring to have it only in a single place is clearly better... |
Do we have any guess on how many people we are going to break with this? Trying to search for the use of this, it seems a zillion people have put up repos which include their virtual environments (https://github.com/search?q=required_interactive_framework&type=Code fromhttps://github.com/search?l=Python&q=class+_BackendQT4Agg%28_BackendQT5Agg%29%3A&type=Code it looks like there are 3k+ vendored versions of mpl on github that is .... something...). The problem with this is API change is that there is (currently) no clean way to warn it and it is an immediate |
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.
I have reservations about removingrequired_interactive_framework at the module level. Enough to not "approve", but not enough to block merging.
required_interactive_framework is very recent: it was only added in 3.0 (in#11520), so September 2018. Most likely, it is only used by people writing cross-backend code, so likely libraries; but how many people are there writing libraries that already require mpl3.0+? It is also quite difficult to use correctly right now (as seen by the dance with Currently you have milestoned 3.2 for September 2019, so let's assume it will come out at some point before the end of the year. Py3.6 was released late december 2019 so 3.2 will almost certainly still support it, so we wouldn't be able to put in a proper deprecation before mpl3.3 at the earliest (say by mid-to-early 2020), which means the attribute will have another 6-9 months to pick up more users who will be affected by its removal. In other words the choice is between affecting a smaller number of users right now, and a potentially larger number next year. |
I am convinced that this is likely not actually used and safe to pull the band-aid off. |
Uh oh!
There was an error while loading.Please reload this page.
In mpl 3.0, the required_interactive_framework attribute was added to
backend modules. However, it is sometimes required to access it from a
canvasinstance (e.g. in _fix_ipython_backend2gui), but the pattern
sys.modules[cls.__module__].required_interactive_frameworkis brittle(e.g. it is broken by third-party subclasses).
Instead it makes more sense to put the attribute on the canvas class;
one can easily and robustly go from the backend module to the canvas
class by accessing the FigureCanvas attribute.
xrefhttps://gitter.im/matplotlib/matplotlib?at=5ced522cef853135c83ae7d6,#14426.
Milestoned as 3.2 as the API is still quite new so the earlier we fix it, the better.
PR Summary
PR Checklist