figure attribute and canvas property

[fariza]

Adding figure attribute (property and setter)

Keeping a canvas property for pyplot stuff that uses direct acces to figuremanager.canvas, this could be easily removed within pyplot

[fariza]

The errors seem unrelated

[OceanWolf]

Yes, the same errors as I have in MEP27, just put in a fix for that...

[OceanWolf]

@fariza I think this will pass. I will give it a quick check over when I come back this evening and merge if okay and then I will work on the the Tom's nits.

Do you want to work on the example(s) as you know what you want to see. Also any idea how to fix the bug in the example? Basically a focus bug, with elements to the left, above of GTK gaining focus first, which means they get the keyboard events and not the canvas -> toolmanager, until you tab focus to the canvas.

On the stable release if you press tab the canvas looses focus, but I see that as a UI issue for another PR. Just want to make sure it has the correct focus if it has no user interaction...

[OceanWolf]

Very nice, I especially like how you use add in the canvas property. It keeps it the same, yet adds in an extra layer of validation meaning one can't change the figure and canvas independently :).

[OceanWolf]

This line looks redundant... this gets set with the next line.

Also do we want to have no figure?

[fariza]

Is not that we want to have no figure (when the figure will be changeable we'll want that option).
In general, I like to declare all the attributes at __init__ and I am not doing self._figure = self._set_figure(figure) because I want to transform that latter into a property setter.

[fariza]

In this moment I didn't introduce the possibility of FigureManager with figure=None.
I was planning on adding that latter (when changeable figures), but if you preffer, I can do it right now.

[OceanWolf]

@fariza Answering to your reply in the code and the comment above.

For me you already have introduced the possibility of figure=None from __init__. I prefer strict typing, if we say that it can equal None as we do in __init__ then we should keep that as a possibility throughout the rest of the code (the places I marked where the code will break).

I wondered what we would want to do with a figure=None, would that destroy the FigureManager?

I presume you mean you will later change the code in init to use getters/setters to self.figure = figure.

Maybe just convert the = None to a FIXME comment saying that we will turn it the next line into a setter method later?

[fariza]

what about just putting the setter in place right now? as you said self.figure = figure.

figure=None... I can image a scenario where you might want to configure your FigureManager before setting the figure in it. So I don't think destroy is the right choice.

[OceanWolf]

Okay, perhaps with a NotImplementedError if already set?

[fariza]

Now with a setter called at __init__, I don't think we have any more problems

[OceanWolf]

Nice, I will give it a quick test locally and merge 😄. Then I will work on improving the tool searching strategy using MEP27 (and then I think I can merge #2 directly into the main branch :).

So will you do a PR for examples? I don't mind if you don't, I just want to know if you will do it. Thanks :)

[fariza]

I want to work on the examples, but I don't know if I'll have the time today or tomorrow

[OceanWolf]

Okay, great, it just gives me one less thing to think about.