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

backend wx and wxagg broken by #1125 #1218

Closed
efiring opened this issue Sep 8, 2012 · 5 comments
Closed

backend wx and wxagg broken by #1125 #1218

efiring opened this issue Sep 8, 2012 · 5 comments
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.

Comments

@efiring
Copy link
Member

efiring commented Sep 8, 2012

Initialization of FigureCanvasWx takes 4 arguments: self, parent, id, figure. The other interactive backends take only self, figure. Change #1125 incorrectly assumes the same signature is used, so it fails on wx and wxagg. This is in addition to the problem with the tkagg backend noted in #1212.

@efiring
Copy link
Member Author

efiring commented Sep 8, 2012

I think the problem here is related to the problem in #1212: in backend_tkagg, the FigureCanvas needs a master kwarg which it is not getting, hence what should have been the master appears as an extra window.

@dmcdougall
Copy link
Member

As I see it, there are three options to resolve this:

  1. Do not support backends that don't have the __init__(self, figure) signature.
  2. Change, for example, the wxagg backend to pass defaults for its parent and id keyword arguments, thus bringing its call signature inline with __init__(self, figure).
  3. Change the call signature of Figure() to pass id and parent to the backend.

Implementing 1): This is the easiest, but users of these backends do not benefit from the reduced boilerplate code. Having said that, the reduced boilerplate code was designed to achieve two things: a) to make it more convenient to set up figures in an object-oriented fashion; and b) to make the backend as transparent as possible to the user.

Implementing 2): This is the hardest, and is a backwards-incompatible change if not done properly. Also, I am not sure (and anybody that uses the wxagg backend please chime in here) it is possible to know what 'default' id and 'default' parent are. Before #1125, was there some other wx-related widget set up to create a parent and an id?

Implementing 3): This makes the figure less backend-agnostic. Though, it would still allow for less boilerplate than 1). A downside here is the possibility of littering the front-end Figure() method with backend-specific parameters as more and more backends are developed. Having said that, this could be kept to a minimum by letting Figure() take only only one kwarg, backend_kwargs, say. This would be a dictionary of backend-specific parameters (id and parent in the case of wxagg) that are passed to the relevant canvas object. Another downside of this is that the Figure class would become less backend-agnostic, what with having to pass on backend-specific parameters.

Ideally, I'd like every user to benefit from reduced boilerplate code, and option 3) achieves this. However, it might be an idea to implement 1) and perhaps discuss further and what needs to happen to support all backends more cleverly. Perhaps an MEP is needed to make the interface for the backends uniform.

@efiring
Copy link
Member Author

efiring commented Sep 8, 2012

The default option is to revert the change. I think the only other possibly viable option is a variation on your option 2, if it turns out to be possible to refactor the tk and wx backends so that their FigureCanvas initializer is functionally identical to that in the other GUI backends. I suspect that sorting this out will take more time than we have prior to the first 1.2 release, so reverting #1125 will be needed. @dmcdougall, would you make a PR to do that, please?

@efiring efiring closed this as completed in 8b8d24f Sep 8, 2012
@efiring
Copy link
Member Author

efiring commented Sep 8, 2012

@dmcdougall, I decided to go ahead and revert this myself. It was not acceptable to leave wx* and tkagg broken in master, and I think that achieving your aim, if possible at all, will require substantial study and perhaps refactoring. There is a reason for pyplot: it really does hide quite a bit of backend variation and messiness.
In any case, we need to get out a solid v1.2 ASAP. There is time, and plenty of scope, for improvements targeted at v1.3.

@dmcdougall
Copy link
Member

@efiring That's fine. It's good to know why this didn't work. Thanks for sorting it out and sorry for causing a bit of a mess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

No branches or pull requests

2 participants