BHoM Tkinter bug fixes and improvements#214
Conversation
…tring identifiers
wired previous unique methods in base class one to ensure backwards compatibility
|
@BHoMBot check required |
|
@Felix-Mallinder to confirm, the following actions are now queued:
|
Tom-Kingstone
left a comment
Tom-Kingstone
left a comment
There was a problem hiding this comment.
Initial review from cursory speed read, some things I noticed that I think need changing.
| except Exception: | ||
| pass |
There was a problem hiding this comment.
| except Exception: | |
| pass | |
| except Exception: | |
| CONSOLE_LOGGER.error("An error occurred when handling an `on_change` event.", exc_info=1) |
please log exceptions
| try: | ||
| return mpl.colormaps[name] | ||
| except KeyError: | ||
| return None |
There was a problem hiding this comment.
| try: | |
| return mpl.colormaps[name] | |
| except KeyError: | |
| return None | |
| return mpl.colormaps.get(name) |
| self._update_cmap_sample() | ||
| return | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
please log exceptions
|
|
||
| def set(self, value: str): | ||
| """Set the selected value. | ||
| """Set the selected value. Silently ignores values not in the current options. |
There was a problem hiding this comment.
please don't make this silent, it is better to have logs for such things (even if they are only for debug)
| numeric_val = self._value_type(raw) | ||
| except (ValueError, TypeError): | ||
| label = "integer" if self._value_type == int else "number" | ||
| return getattr(self, "apply_validation")((False, f"Must be a valid {label}.", "error")) |
There was a problem hiding this comment.
investigate this, why are we using getattr? surely self.apply_validation(...) would work here?
|
@BHoMBot check installer |
|
@Tom-Kingstone to confirm, the following actions are now queued:
|
|
The check |
applying suggestion mainly to trigger bot check. Most likely the try/except block is going to be removed entirely
| ANALYTICS_LOGGER.info( | ||
| json.dumps(exec_metadata, default=str, indent=None) | ||
| ) | ||
| try: |
There was a problem hiding this comment.
potentially remove this try/except as it was originally added when you were locally testing in the wrong environment.
|
@Tom-Kingstone to confirm, the following actions are now queued:
|
2 participants
Issues addressed by this PR
Closes #213
General upgrade to the BHoM Tkinter tools, prompted by bugs in aesthetic and functionality issues identified in dependant toolkit(s).
Overview:
on_changecallback implemented at toplevel, inbhom_base_widget, for consistent action of widgets in a formTest files
~\Python_Toolkit\Python_Engine\Python\testsChangelog
Additional comments