I know it may be asking a bit much but I’m looking for someone familiar with wxPython who would review the code of a small app (<400 LoC). It all runs well now but I’m really eager to learn and improve. I’m an experienced Java & web developer but Python is new to me.
While I could ask someone at work who’s familiar with Python I feel it’s necessary to have wxPython know-how specifically.
Greetings Marcel,
It is a very short program, and if it is not proprietary, you could simply post it in a web repository like dropbox and
simply say: I’m new to wxPython, and I wrote a small app and would appreciate opinions on it so I can improve.
And post a link to your code.
I’m sure there are many here who would review a short program.
Rufus
On Jan 12, 2017, at 2:38 AM, Marcel Stör marcel@frightanic.com wrote:
I know it may be asking a bit much but I’m looking for someone familiar with wxPython who would review the code of a small app (<400 LoC). It all runs well now but I’m really eager to learn and improve. I’m an experienced Java & web developer but Python is new to me.
While I could ask someone at work who’s familiar with Python I feel it’s necessary to have wxPython know-how specifically.
–
You received this message because you are subscribed to the Google Groups “wxPython-users” group.
To unsubscribe from this group and stop receiving emails from it, send an email to wxpython-users+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
I would just post the code using Github’s gist or use pastebin or even create a github project. They’re a bit simpler for sharing code and updating it.
On Thursday, January 12, 2017 at 11:18:22 AM UTC-6, Rufus wrote:
Greetings Marcel,
It is a very short program, and if it is not proprietary, you could simply post it in a web repository like dropbox and
simply say: I’m new to wxPython, and I wrote a small app and would appreciate opinions on it so I can improve.
And post a link to your code.
I’m sure there are many here who would review a short program.
Rufus
Rufus, Mike, thanks for your encouraging comments, I appreciate that.
I wanted to wait with posting the link to the GitHub repository until I got the first positive replies. I wanted to rule out that some people would think of this call for help as a disguised way of promoting my app.
https://github.com/marcelstoer/nodemcu-pyflasher
entry point: nodemcu-pyflasher.py
build* files are for PyInstaller
Let me know if you need more information.
Hi Marcel,
This piqued my interest as I have a box of ESP devices at home that I’ve never gotten to work. 
Anyhow, a quick once over turned up two comments:
The tab order of widgets in wx is determined by their creation order, so since you are creating the “Flash” button and console before any of the radio buttons, the focus seems weird when you use the tab key to navigate the dialog. Easy to fix (if you care), simply move the button and console widget creation down after the radio button groups.
I usually create radio buttons as a group, and bind an event handler to each button to avoid messing around with logic to figure out which button got pushed when and where. For example, here’s how I’d rewrite your baud rate handler (with minimal changes to anything else).
serial_boxsizer.Add(reload_button, 0, wx.ALIGN_RIGHT, 20)
def OnBaudChanged(event):
button = event.GetEventObject()
if button.GetValue():
self.__config.baud = button.rate
print("Set baud rate to", self.__config.baud)
def AddButton(sizer, rate):
button = wx.RadioButton(panel, name="baud-%d" % rate, label="%d" % rate, style=wx.RB_GROUP if rate == 9600 else 0)
button.rate = rate
button.SetValue(rate == self.__config.baud)
button.Bind(wx.EVT_RADIOBUTTON, OnBaudChanged)
sizer.Add(button)
sizer.AddSpacer(10)
baud_boxsizer = wx.BoxSizer(wx.HORIZONTAL)
for rate in 9600, 57600, 74880, 115200, 230400, 460800, 921600:
AddButton(baud_boxsizer, rate)
flashmode_boxsizer = wx.BoxSizer(wx.HORIZONTAL)
HTH,
Eric
I appreciate your feedback Eric, thanks
This piqued my interest as I have a box of ESP devices at home that I’ve never gotten to work.
If you ever get around to unboxing them that might be my way of paying you back. While I don’t have an electronics background I have been current NodeMCU firmware team since summer 2015. So, I know a thing or two about those devices.
Anyhow, a quick once over turned up two comments:
- The tab order of widgets in wx is determined by their creation order, so since you are creating the “Flash” button and console before any of the radio buttons, the focus seems weird when you use the tab key to navigate the dialog. Easy to fix (if you care), simply move the button and console widget creation down after the radio button groups.
I definitely do care and when I build web forms I pay attention to this. Don’t know why I didn’t test this here. Will fix.
- I usually create radio buttons as a group, and bind an event handler to each button to avoid messing around with logic to figure out which button got pushed when and where. For example, here’s how I’d rewrite your baud rate handler (with minimal changes to anything else).
serial_boxsizer.Add(reload_button, 0, wx.ALIGN_RIGHT, 20)
def OnBaudChanged(event):
button = event.GetEventObject()
if button.GetValue():
self.__config.baud = button.rate
print("Set baud rate to", self.__config.baud)
def AddButton(sizer, rate):
button = wx.RadioButton(panel, name="baud-%d" % rate, label="%d" % rate, style=wx.RB_GROUP if rate == 9600 else 0)
button.rate = rate
button.SetValue(rate == self.__config.baud)
button.Bind(wx.EVT_RADIOBUTTON, OnBaudChanged)
sizer.Add(button)
sizer.AddSpacer(10)
baud_boxsizer = wx.BoxSizer(wx.HORIZONTAL)
for rate in 9600, 57600, 74880, 115200, 230400, 460800, 921600:
AddButton(baud_boxsizer, rate)
flashmode_boxsizer = wx.BoxSizer(wx.HORIZONTAL)
I was definitely surprised to see that a radio button group is only a pseudo/virtual widget in wxPython (I know about RadioBox, though). So, dealing with its event handler felt really odd. I’ll refactor as you suggested.
What were those other “minimal changes to anything else”? The function names and their visibility?
That’s one other thing that I spent some time thinking and reading about. Knowing that I’m coming from Java that has private, (package) protected and public methods you may not be surprised I added those __ to all “internal” functions. I also used the all-lower snake case naming pattern rather than lower camel case as is common in Java. Your code and the wxPython code uses upper camel case. Python styling guidelines seem to be rather liberal?
I found your code very readable. Great job. Superior even, for someone new to Python and wxPython.
When I ran the code on my Mac, the menus didn’t drop down properly.
Other than that, the only pedantic thing I’d say is not to use “file” as an attribute name. It ends up being
highlighted (as a keyword) in my editor.
Rufus
On Jan 12, 2017, at 2:38 AM, Marcel Stör marcel@frightanic.com wrote:
I know it may be asking a bit much but I’m looking for someone familiar with wxPython who would review the code of a small app (<400 LoC). It all runs well now but I’m really eager to learn and improve. I’m an experienced Java & web developer but Python is new to me.
While I could ask someone at work who’s familiar with Python I feel it’s necessary to have wxPython know-how specifically.
–
You received this message because you are subscribed to the Google Groups “wxPython-users” group.
To unsubscribe from this group and stop receiving emails from it, send an email to wxpython-users+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
I found your code very readable. Great job. Superior even, for someone new to Python and wxPython.
Thanks for your kind words.
When I ran the code on my Mac, the menus didn’t drop down properly.
I know, it’s a bit irritating but I don’t know how to fix that. On Mac both “Exit” and “About” are placed in that synthetical (i.e. generated) ‘Python’ menu rather than in ‘File’ and ‘Help’ respectively.
Other than that, the only pedantic thing I’d say is not to use “file” as an attribute name. It ends up being
highlighted (as a keyword) in my editor.
Ok, good to know. Which one is that? I use PyCharm CE.
When I ran the code on my Mac, the menus didn’t drop down properly.
I know, it’s a bit irritating but I don’t know how to fix that. On Mac both “Exit” and “About” are placed in that synthetical (i.e. generated) ‘Python’ menu rather than in ‘File’ and ‘Help’ respectively.
I just had to look into that menu issue on the Mac, because I would be tripped up by the same thing if I were to create menus (all my wxPython
programs have been under Windows, until I had to steal my son’s Macbook Pro when my laptop became out of commission)
I found out when you use the stock ID's (wx.ID_ABOUT, wx.ID_EXIT, wx.ID_PREFERENCES, etc.) it basically ignores the placement in your menu and puts them in the Python menu, as
you say.
But if you let it create an ID with wx.ID_ANY, your menu choice will be where you expect them. It does make it seem like you'd have to bind both to the same event handler
to be able to use both.
Other than that, the only pedantic thing I’d say is not to use “file” as an attribute name. It ends up being
highlighted (as a keyword) in my editor.
Ok, good to know. Which one is that? I use PyCharm CE.
I'm using WingIDE and really like it.
Cheers.
On Jan 13, 2017, at 4:53 PM, Marcel Stör marcel@frightanic.com wrote:
I meant to append these pertinent URLs:
http://www.macworld.com/article/3065998/os-x/the-power-of-os-xs-menu-bar.html
https://wiki.wxpython.org/Optimizing%20for%20Mac%20OS%20X
On Jan 14, 2017, at 12:53 AM, Rufus Smith rufusvsmith@gmail.com wrote:
On Jan 13, 2017, at 4:53 PM, Marcel Stör marcel@frightanic.com wrote:
When I ran the code on my Mac, the menus didn’t drop down properly.
I know, it’s a bit irritating but I don’t know how to fix that. On Mac both “Exit” and “About” are placed in that synthetical (i.e. generated) ‘Python’ menu rather than in ‘File’ and ‘Help’ respectively.
I just had to look into that menu issue on the Mac, because I would be tripped up by the same thing if I were to create menus (all my wxPython
programs have been under Windows, until I had to steal my son’s Macbook Pro when my laptop became out of commission)
I found out when you use the stock ID’s (wx.ID_ABOUT, wx.ID_EXIT, wx.ID_PREFERENCES, etc.) it basically ignores the placement in your menu and puts them in the Python menu, as
you say.
But if you let it create an ID with wx.ID_ANY, your menu choice will be where you expect them. It does make it seem like you’d have to bind both to the same event handler
to be able to use both.
Other than that, the only pedantic thing I’d say is not to use “file” as an attribute name. It ends up being
highlighted (as a keyword) in my editor.
Ok, good to know. Which one is that? I use PyCharm CE.
I’m using WingIDE and really like it.
Cheers.
I found out when you use the stock ID’s (wx.ID_ABOUT, wx.ID_EXIT, wx.ID_PREFERENCES, etc.) it basically ignores the placement in your menu and puts them in the Python menu, as you say.
But if you let it create an ID with wx.ID_ANY, your menu choice will be where you expect them.
https://wiki.wxpython.org/Optimizing%20for%20Mac%20OS%20X explains that, thanks for this link. The fact that this article a) looked vaguely familiar and b) shows the same menu code as I have it lets me assume I must have used it as a blue print. I believe the wxPython default behavior for wx.ID_ABOUT, wx.ID_EXIT etc. does make a lot of sense as it respects and adjusts to platform defaults.
However, two things make this look odd currently in my application:
As there’s no self-contained binary of my application yet (due to Packaged wxPython Hello-World crashes on macOS · Issue #2355 · pyinstaller/pyinstaller · GitHub) it uses Python as a boot loader. Thus, the macOS “application menu” is called “Python” rather than . Time will fix this I hope…
My app has only two menu items (exit and about). Since both of are moved to the macOS “application menu” the two designated menus are empty. I can only fix this by adding more features I guess 
If you ever get around to unboxing them that might be my way of paying you back. While I don’t have an electronics background I have been current NodeMCU firmware team since summer 2015. So, I know a thing or two about those devices.
I’ve got a test setup, but never got it to talk over the serial line. I’ve got something miswired somewhere, so there’s really nothing anyone else can do, I just need to put some time into figgerin’ it out. Thanks for the offer, though…
I definitely do care and when I build web forms I pay attention to this. Don’t know why I didn’t test this here. Will fix.
Yeah, that’s a quirk in some gui packages: some use the layout manager to dictate tab navigation order, some use widget creation order, just one of those things you have to know/remember.
What were those other “minimal changes to anything else”? The function names and their visibility?
I’m accustom to large applications (our big product has 80k lines just for the gui code), so I would (did) derive a real RadioBox class that completely encapsulates the RadioButtons and all their idiosyncracies. This would just add needless complexity to your program, which is nice and compact. If you intend for it to grow into some behemoth, do-it-all, uploader/downloader/browser, then it might be worth all the extra structure, but for a simple wrapper on a command-line utility, I think what you’ve done is just great.
That’s one other thing that I spent some time thinking and reading about. Knowing that I’m coming from Java that has private, (package) protected and public methods you may not be surprised I added those __ to all “internal” functions.
It’s more common to see just a single underscore, which is an idiom that says “hey, this is private, use it at your own risk”. The double underscores are usually reserved for the more emphatic “don’t even think about it” case. Pythonic programming is much more about adhering to a gentleperson’s agreement than dictating terms. When you do a production code review, you always flag it when someone uses an “_attribute” out of context, but during debugging it can save you all sorts of hoop-jumping. It’s just part of the culture.
I also used the all-lower snake case naming pattern rather than lower camel case as is common in Java. Your code and the wxPython code uses upper camel case. Python styling guidelines seem to be rather liberal?
Right, I use PEP-8 conventions (lower snake) for most code, but when dealing with “external” packages, I use the package conventions. Again, it’s idiomatic to do so, and deviations are documentation. (I just mentioned this one on python-ideas, but it applies everywhere as far as I’m concerned: Stupid Python Ideas: Why following idioms matters ) So, if I see lower-snake, I think, “ok, this is pure Python, nothing to do with WX”. When I see lower-camel, then I think, “this is an application-specific extension to the WX widget (maybe it talks to the database);” upper-camel means, “this is WX-like, either an override and in any case it’s not application-specific.”
On Friday, January 13, 2017 at 1:43:57 PM UTC-8, Marcel Stör wrote:
Actually, there's no reason you can't define TWO menu items, one with the stock ID and one with a unique ID and bind them to the same handler, is there?
On Jan 14, 2017, at 10:34 AM, Marcel Stör <marcel@frightanic.com> wrote:
2. My app has only two menu items (exit and about). Since both of are moved to the macOS "application menu" the two designated menus are empty. I can only fix this by adding more features I guess