Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

wx backend changes for wxPython Phoenix #1974

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

wernerfb commented May 3, 2013

I used embedding_in_wx5 for testing and tested against 2.8.12 and 2.95 preview classic and 2.95 Phoenix (snapshot 2.9.5.81 r73883).

Worked against tag 1.2.0 which is what I have installed (Win 7), hope this is fine.

@pelson pelson commented on the diff May 7, 2013

examples/user_interfaces/embedding_in_wx5.py
import wx
-import wx.aui
+print wx.VERSION_STRING
+
+import wx.lib.agw.aui as aui
@pelson

pelson May 7, 2013

Member

Does this work with v2.8?

@wernerfb

wernerfb May 7, 2013

Contributor

On 07/05/2013 12:52, Phil Elson wrote:

In examples/user_interfaces/embedding_in_wx5.py:

import wx
-import wx.aui
+print wx.VERSION_STRING
+
+import wx.lib.agw.aui as aui

Does this work with v2.8?

I testet with 2.8.12.1, as per changes.html in 2.8.12:

2.8.9.2

  • 16-Feb-2009

Added the wx.lib.agw package ........

So, only as of 2.8.9+ will it work, if that is not acceptable that one
could do:

     if 'phoenix' in wx.PlatformInfo:
            import wx.lib.agw.aui as aui
     else:
            import wx.aui as aui

Werner

@efiring

efiring May 13, 2013

Owner

A more general way to handle it would be

try:
    import wx.lib.agw.aui as aui
except ImportError:
    import wx.aui as aui

@pelson pelson commented on the diff May 7, 2013

lib/matplotlib/backends/backend_wx.py
@@ -666,8 +666,6 @@ class FigureCanvasWx(FigureCanvasBase, wx.Panel):
wx.WXK_DELETE : 'delete',
wx.WXK_HOME : 'home',
wx.WXK_END : 'end',
- wx.WXK_PRIOR : 'pageup',
@pelson

pelson May 7, 2013

Member

Looks like these don't exist any more in wx?

@wernerfb

wernerfb May 7, 2013

Contributor

Hi,

On 07/05/2013 12:58, Phil Elson wrote:

In lib/matplotlib/backends/backend_wx.py:

@@ -666,8 +666,6 @@ class FigureCanvasWx(FigureCanvasBase, wx.Panel):
wx.WXK_DELETE : 'delete',
wx.WXK_HOME : 'home',
wx.WXK_END : 'end',

  •    wx.WXK_PRIOR           : 'pageup',
    

Looks like these don't exist any more in wx?

Prior yes, the others are still there:
wxpython.org/Phoenix/docs/html/KeyCode.enumeration.html

Werner

@pelson pelson commented on the diff May 7, 2013

lib/matplotlib/backends/backend_wx.py
+ self.wx_ids[text] = int(wx.NewId())
+ if 'phoenix' in wx.PlatformInfo:
+ if text in ['Pan', 'Zoom']:
+ self.AddTool(self.wx_ids[text], label=text,
+ bitmap=_load_bitmap(image_file + '.png'),
+ bmpDisabled=wx.NullBitmap,
+ shortHelpString=text,
+ longHelpString=tooltip_text,
+ kind=wx.ITEM_CHECK)
+ else:
+ self.AddTool(self.wx_ids[text], label=text,
+ bitmap=_load_bitmap(image_file + '.png'),
+ bmpDisabled=wx.NullBitmap,
+ shortHelpString=text,
+ longHelpString=tooltip_text,
+ kind=wx.ITEM_NORMAL)
@pelson

pelson May 7, 2013

Member

Looks like "kind" is the only difference here - probably worth putting "kind" in a variable and having the AddTool outside of the if statement.

wernerfb added some commits May 10, 2013

@wernerfb wernerfb - wxPython Phoenix 165f2a2
@wernerfb wernerfb Merge remote-tracking branch 'remotes/origin/Branch_v1.2.0' into v1.2.x
Conflicts:
	examples/user_interfaces/embedding_in_wx5.py
	lib/matplotlib/backends/backend_wx.py
4d5ae20
Owner

efiring commented May 11, 2013

@wernerfb, it is good to see this work.
I think it should target 1.3 (the master git branch); the 1.2.x branch is for bugs, and this goes well beyond that. I suspect your changes could be extracted as a diff and then applied as a patch to a feature branch based on the current master, so they would be merged into a single clean commit. When you are working on a feature branch, it is better not to merge anything into it from the main branches. Instead, if your feature branch does not merge cleanly into its target, because of changes in the latter, it is best to rebase it so that it will merge cleanly.

Contributor

wernerfb commented May 12, 2013

Hi Eric,

On 11/05/2013 23:53, Eric Firing wrote:

@wernerfb https://github.com/wernerfb, it is good to see this work.

I think it should target 1.3 (the master git branch); the 1.2.x branch
is for bugs, and this goes well beyond that. I suspect your changes
could be extracted as a diff and then applied as a patch to a feature
branch based on the current master, so they would be merged into a
single clean commit. When you are working on a feature branch, it is
better not to merge anything into it from the main branches. Instead,
if your feature branch does not merge cleanly into its target, because
of changes in the latter, it is best to rebase it so that it will
merge cleanly.

I am very new to git and I can't build mpl locally, that is why I did it
against the tag for 1.2.0.

If I understand your suggestion correctly, I should use "rebase" on my
local clone to 1.3, push the change and I assume I would then need to do
another pull request?

Werner

Owner

efiring commented May 13, 2013

Werner, yes, you would rebase on current master and make a new pull request with master as the base. Be sure to use a feature branch, however--see http://matplotlib.org/devel/index.html for some workflow tips. I think it would be by far the best if you could build mpl locally, so you could fully test your changes in the normal way. Also, because this involves testing with a very new and not widely distributed version of wx, it might take a while before others are able to test it, so maybe the 1.3 target is too optimistic.

There have been quite a few changes in backend_wx* from 1.2.x to master, and it looks like more are needed even without the Phoenix compatibility changes. I'm not sure whether it would be better to try to use git to rebase, or whether it might be easier to essentially re-do the changes directly on master. If you run into trouble with the former, you can always fall back on the latter.

I think there may be quite a bit of testing and work required to make sure these sorts of changes work on sufficiently old versions of wx--and some thought may need to go into the question of how old a version we need to support here.

Contributor

wernerfb commented May 13, 2013

It looks like the rebase was clean, I'll push that in a little while.

I don't think I will find time in the near future to figure out what I need to build 1.3 locally (don't I need a C compiler etc etc), are there preview builds for win 32 I could maybe use?

It would be great if this would make it into 1.3 and as the latest changes should be compatible with wxPython 2.8+ classic and 2.9.5 Phoenix this should be possible no? At least this way it gets more testing, even if mostly with wxPython classic.

@efiring efiring referenced this pull request May 13, 2013

Closed

- wxPython Phoenix #1995

Owner

efiring commented May 13, 2013

I am closing this because it is replaced by #1995.
I agree with your comment above, @wernerfb, that #1995 appears to be fully compatible with non-phoenix wx, and if so, it should be possible to merge it quite promptly.

@efiring efiring closed this May 13, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment