making figuremanager figure centric instead of canvas centric

[fariza]

Same idea and implementation as with my "multifigure-toolbar" PR but just implementing the stuff in the FigureManager and WindowGTK3.

Please run the example examples/user_interfaces/reuse_figuremanager.py just pressing f once will switch the figure.

The toolbar and toolmanager don't support the FigureManager change of figures, but I already know and have implemented that in my previous PR. I just want to know if you like this idea

[fariza]

Same here with the additional commits, I don't get it.

[OceanWolf]

I haven't had a good look through this, but in general I like it, I presume this helps pave the way for multi-figure, 😄.

[fariza]

Yes for multi-figure but for really actually reusable components

[OceanWolf]

I like the idea, but right now I feel more concerned with getting MEP27 reviewed and out so we can work on the other backends, and I don't see this as a blocker, i.e. it doesn't add a feature that we can't add later without any impact on users.

[fariza]

I kind of agree, the only problem, is that if you provide a figuremanager.canvas attribute from the beginning, you will be stuck with providing it forever after.
As you put it before, it is called figuremanager not canvasmanager

[fariza]

Actually I will push a PR for toolmanager that does exactly that, but it will store a figuremanager attribute instead of canvas attribute.

[OceanWolf]

Ahh, understand now what you mean, I didn't notice that difference before. I kind of agree with you, I mean the only reason for having figuremanager.canvas (and my only concern) comes purely from backward-compatibility... before we have many different figman classes all with a figcanvas, now just one, but it still behaves in the same way, only the constructor changes...

If BC ain't a problem here, then fine, but lets make sure it plays nice with the rest of the codebase as well...

[fariza]

I would go for having figuremanager.figure (property) and not have figuremanager.canvas if the other devs want a canvas we can have it as a property, so we could add a deprecation warning.
But don't add it from the beginning

[OceanWolf]

Okay, fine, I think you have convinced me, give it a rebase and lets see if we get any errors from other parts of the codebase wanting a canvas attribute...

[OceanWolf]

Just taken one more look at this and what do you think about splitting this into two? I.e. we have:

1. Making FigureManager figure centric (figman.canvas -> figman.figure)
2. Replace element / replace figure logic.

From what I see we need to add 1. now, 2. we can add later. Not adverse to 2. just that it creates extra work for the backends and I want to play it super-safe from a next point release deadline point of view from what you write above we can add that part later, probably fine, but I have learnt the hard way not to try and do much.

[OceanWolf]

@fariza Also see my comments in #1 about how to make a PR without any of the weird additional commits ;).

[fariza]

The part that touches every backend is the replace_element. It would be nice if we have it there from the begining and we minimize the rework to be done in every backend.

The replace figure logic... It's already there... if you insist we can leave it out 😢

[OceanWolf]

Sure, I understand that replace_element logic touches every backend, and for that reason I suggest we leave it out, we can add this logic in a single PR as only a simple addition to each backend, we should still have time for it to catch the next point release, but I want to stay on the safe side. Too often I say "it will be fine we have plenty of time", and then end up having a panic attack as the deadline draws near (I cook for 20 people once a month.

I think we can get everything bar this done in two weeks, that should give us a two week margin for error for slow release checks (plus I want to make another PR to refactor out the Save Tool out of the backend and into backend.tools, in the way GTK3 does it).

[fariza]

Ok, I'll change that to leave it out.

[OceanWolf]

Great, how easy to do it in a fresh PR following the method in #1?

[fariza]

Done, check PR #6