FIX: MPL should not use new tool manager unless explicited asked for. Closes #7404

[OceanWolf]

A quick fix to close the bug, but we probably need to come up with a better PR strategy for this.

@fariza Any other ideas/alternatives to this patch? IIRC we compromised on this before saying that toolbar == None should not prevent the tools from working with shortcut keys, just that the toolbar would not get displayed.

Of course the main MEP27 PR fixes this straight away as we decided the new toolmanager should only work with the new figuremanager code, so this got taken care over there yonks ago.

[fariza]

This is the easiest fix.
I would prefer to change the warning to use our internal warnings that we can disable by config.
I'll give it a try this afternoon to see how doable it is.

[OceanWolf]

Internal warnings? Great, I was thinking about that yesterday that we should do something like:

def mpl_warn(message, warning_type):
  if warning_type in std_warn_list:
    warnings.warn(message)

Does our internal warnings do something like that?

And yes this fix brings it on par with how the MEP27 branch works now (i.e. prior to the removal of the FigureManagerX classes), with the exception that in the MEP27 branch the user can still create an MEP22 enabled FigureManager without a toolmanager by having toolbar set to None and explicitly making the new FigureManager class with:

fig = Figure()
figmgr = FigureManager(fig, title)

[fariza]

Maybe for simplicity, we should just use your fix, and with MEP27 we can fix the warnings handling

[OceanWolf]

Well, with MEP27 too late to fix warning handling as we will have hopefully worked out all the experimental nature of this by then... with all backends working.

I think we should put the warnings in as a separate PR as they don't really relate to MEP27 specifically, nor anything else.

[fariza]

ok, we fix this with MEP27 and the warnings in a separate PR

[tacaswell]

Please remember to backport

[OceanWolf]

@tacaswell I thought I had already asked you here (obviously the comment never sent) about whether or not we should keep this PR open to backport.

[NelleV]

We are starting to have a lot of merge conflicts in backports…
This cherry-pick does not apply cleanly. I'll create a PR with the backport.

[OceanWolf]

Backport where? We don't have a 2.0.1 branch yet, as far as I can see... confused.

[NelleV]

the 2.0.1 goes into v2.x. All minor release share the same branch.
But this has diverged from v2.x and doesn't need to be backported.

[OceanWolf]

Okay, a bit weird though since we haven't released 2.0 yet and we won't backport 2.1 as that will come direct from master, so to me it makes more sense to have a 2.0.x branch once 2.0 gets released and then a 2.1.x branch after 2.1 gets released, or do I miss something here?

[tacaswell]

#7404 is reported against 1.5.3 so presumably it is broken in 2.x as well?

What you suggest does make sense under normal circumstances, but for now we are in a weird state of not having a 1.5.x and 2.x being a 'live' branch as well as master so 2.x is serving as both the target of 'new' stuff for 2.0 and all of the bug-fixes that are reported against older version.

[OceanWolf]

Yes, broken MPL 1.5+.

Okay, I find that confusing. Did that get written somewhere up how this branches currently work and I missed it?

So as I understand it:

• 1.5.x no longer exists because we don't want to release another bug fix before 2.0
• 2.0 will launch from the 2.x branch which in turn branched from 1.5.x? This kind of makes sense apart from I thought 2.0 was supposed to only include the style changes, no features (and I presumed that meant bug fixes as well).
• 2.1 will come from master and then we will branch that to create a 2.1.x branch if we have bug-fixes.

I would have expected it to flow more logically, like we have 1.5.x bug fix branch from which we branch a 2.0.x branch which the style changes go into. Bug fixes go into 1.5.x, then every time we do a bug-fix release we rebase the 2.0 branch onto the 1.5.x branch. When we release 2.0 we tag in the 2.0.x branch and then do a final rebase from the 1.5.x branch to make sure we have all the bugs that didn't make in into a 1.5.x release ready for our first 2.0.x bug fix release.

[QuLogic]

Public commits in a popular repo like this should not be rebased. You are essentially saying the same thing, except we merge instead of rebase.

[tacaswell]

Bug fixes can always go in ;) There have been a couple of emails to the mailing list about it, but I do not think it was ever explicitly stated that bug-fixes and documentation were fair-game to backport.

We can not rebase any branch on the mpl org, too many people treat it as an 'upstream'

[QuLogic]

Also, the 2.0.1 milestone is code for a nice-to-have, non-blocker issue, but it's really just the same as 2.0 once it's merged. No-one has been updating them, but I might just do that after a release is made.

[OceanWolf]

Ahh okay, confusion solved.

[QuLogic]

#7404 was reported against 1.5.3 and was intended to be backported, but it appears this hasn't been; should it be (to 2.0.1 if necessary)?

[OceanWolf]

Ahh yes, it does need backporting.

[QuLogic]

Backported to v2.x as 7371b52.