Tighten code and add magnification control to the hand panel #142
Conversation
src/instamatic/gui/ctrl_frame.py
Outdated
| ranges = config.microscope.ranges | ||
| modes = list(ranges.keys()) |
There was a problem hiding this comment.
Python always iterates over the dict keys, so it's not necessary to call keys.
| ranges = config.microscope.ranges | |
| modes = list(ranges.keys()) | |
| modes = list(config.microscope.ranges) |
There was a problem hiding this comment.
The panels gets only as much love as time I can spare now, unfortunately.
Also, as import this verse 2 proclaimeth, Explicit is better than implicit
There was a problem hiding this comment.
Nice! I had some minor comments, but otherwise I think these are useful additions!
One thing I noticed reading through the file is the inconsistency of using the event queue or simply changing things directly in self.ctrl. Is that something to consider? I am not familiar with how the GUI is set up, especially regarding multiple threads/communication with servers
src/instamatic/gui/ctrl_frame.py
Outdated
| ranges = config.microscope.ranges | ||
| modes = list(ranges.keys()) | ||
| self.o_mode = OptionMenu(frame, self.var_mode, modes[0], *modes, command=self.set_mode) | ||
| self.o_mode.grid(row=8, column=1, sticky='EW') |
There was a problem hiding this comment.
One thing I noticed when playing around with simulations is that the initial value for the dropdown is not the actual initial value on the microscope (at least using the simulation microscope). I also just noticed that the names of the different modes are inconsistent between FEI and JEOL microscopes, but your implementation here accounts for that.
self.ctrl is only populated in the end of the constructor, so I don't think we can get the initial mode without moving self.ctrl to the start of the constructor.
There was a problem hiding this comment.
@viljarjf Yea, the values were hardcoded here so I suggest pulling them from config. I am not sure why self.ctrl is at the end, I checked and it works at the beginning at least on the simulator. This prevents nice larger changes. @stefsmeets do you remember?
src/instamatic/gui/ctrl_frame.py
Outdated
| self.ctrl.magnification.decrease() | ||
|
|
||
| def reset_stage(self): | ||
| self.ctrl.stage.set(0, 0, 0, 0, 0) |
There was a problem hiding this comment.
ctrl.stage.neutral() also exists, but it does exactly the same
| # Magnification | ||
| Label(frame, text='Magnification', width=20).grid(row=14, column=0, sticky='W') |
There was a problem hiding this comment.
Is there any merit to displaying the magnification value as well? Increasing and decreasing with buttons is the way to go I think, but at least for the sim microscope it is useful to see the actual magnification too
There was a problem hiding this comment.
@viljarjf I absolutely wanted to display this value and the traces of this idea are in the code (see explicit declaration of ranges = config.microscope.ranges dict with this info) but once I started implementing it I noticed this requires a lot of changes, so I backpedaled for now. Maybe I'll add a print at least.
If you directly update |
Co-authored-by: Viljar Femoen <viljar.femoen@hotmail.no>
|
Ok, I applied suggested changes and will be merging tomorrow-ish unless anyone objects. I added print statements on magnification change, |
|
Following #141 testing session, I added another window for stage Z position in line with X and Y. This makes the panel contents slightly wider, but as I discovered is very useful even for simulation. |
Introduces minor changes to the manual control panel aimed to add magnification buttons suggested by @viljarjf in #140 and tighten it a bit, both in terms of code quality/quantity and space taken in the GUI:
EntrytoSpinboxwith 100-nm increment to allow setting with mouse;Note how the refreshed panel has the same height as the old one despite having a new row.
When I started editing I had large changes in mind, but I decided to scale them down and leave the spotlight on the simulator for the time being. On accident, I also found out that repeatedly turning the alpha wobbler on and off consistently crashes Instamatic. Could not fix it quickly, this is a low-priority issue for the future.