wxGUI/history: Display saved region name if stored#7095
Conversation
wenzeslaus
left a comment
wenzeslaus
left a comment
There was a problem hiding this comment.
The comparison needs more reasoning (epsilon, 3D, ...). Similarly, please describe behavior for the different situations.
As for the details here, user may still want to update to the named (saved) region. The GUI element alignment is wrong.
|
Yes, you should also compare the values for 3D. |
|
I know there are still some changes needed, but I wanted to check if I’m heading in the right direction or just overcomplicating things. Currently, I also switched to format="shell" instead of JSON because the JSON output was returning verbose keys like "north" instead of short keys like "n", which simplifies parsing. Additionally, I added translation keys for the new 3D fields, although these can be removed if not needed. From what I could find , when region 3D values aren’t present, they are set to defaults automatically, so converting from a 2D region to 3D will provide reasonable default values right? |
petrasovaa
left a comment
petrasovaa
left a comment
There was a problem hiding this comment.
The order of the items should be 2D first and 3D at the bottom. Now the ordering follows the default g.region print order, but here it would be better to start with the 2D.
Ideally we will get these unified. The idea is to use the json for everything. So the history could be using the g.region format=json for writing instead of the gs.region() function. But maybe we could skip these changes now. |
|
@petrasovaa Can you check this? I have removed |
|
From testing.. : This part works only for saved regions in the current mapset, if you are in another mapset, it fails: _Traceback (most recent call last): self._runCmd(self.GetCurLine()[0].strip()) self.promptRunCmd.emit(cmd={"cmd": cmd, "cmdString": dispatcher.send(signal=self, *args, **kwargs) response = robustapply.robustApply( return receiver(*arguments, **named) lambda cmd: self._gconsole.RunCmd(command=cmd) self._giface.entryToHistoryAdded.emit(entry=entry) dispatcher.send(signal=self, *args, **kwargs) response = robustapply.robustApply( return receiver(*arguments, **named) lambda entry: self.InsertCommand(entry) self.infoPanel.showCommandInfo(command_info) self._updateRegionSettingsMatch() saved_name = self._get_saved_region_name(history_region) saved_region = self.tools.g_region( return self._run_function(tool_name, **kwargs) result = self._run_cmd( result = self.call_cmd( return gs.handle_errors( raise exception(module, code, returncode=returncode, |
| "ewres3": _("3D east-west resolution:"), | ||
| "rows3": _("Number of 3D rows:"), | ||
| "cols3": _("Number of 3D cols:"), | ||
| "depths": _("Number of depths:"), |
There was a problem hiding this comment.
Please check if you include all values for the comparison of saved and history region (I can see that e.g. 3D cells are missing)
|
|
||
| self.region_settings = command_info["region"] | ||
|
|
||
| Display_Order = [ |
There was a problem hiding this comment.
Maybe the Display order could be already part of the TRASLATION_KEYS that could be renamed to REGION_KEYS? Just idea - your current code has basically three different very similar variables here (TRANSLATION_KEYS, Display_Order and valid_keys) so maybe some shortening would be nice. Also what is wrong with region_settings_filter method? As we want to compare mainly numerical values from g.region, I can imagine adding ellipsoid and datum to the filter and get rid of cellls, but otherwise using this filter seems ok for me (maybe I am missing something).
There was a problem hiding this comment.
Hello Linda, apologies for the late response. I think the reason I removed region_settings_filter was that rows3, cols3, and depths caused issues when passed to g.region through the Update current region button. Because of that, I thought we could not use the same filter for both display order and valid keys together. Maybe we can remove valid_keys and use the filter there instead, but I’m not sure how best to handle display order. Do you have a better idea?
There was a problem hiding this comment.
I am open to whatever solution here if it works.
| res = self.tools.g_list(type="region", mapset="*", format="json") | ||
| region_names = [r["name"] for r in res] |
There was a problem hiding this comment.
Likely this needs the full name including the mapset:
| res = self.tools.g_list(type="region", mapset="*", format="json") | |
| region_names = [r["name"] for r in res] | |
| res = self.tools.g_list(type="region", mapset="*", flags="m", format="json") | |
| region_names = [r["fullname"] for r in res] |
|
|
||
| if self._compare_regions(history_region, saved_region): | ||
| return region_name | ||
| except AttributeError: |
There was a problem hiding this comment.
Include ToolError here (from tools).
|
|
||
| self.region_settings = command_info["region"] | ||
|
|
||
| Display_Order = [ |
There was a problem hiding this comment.
This is minor, but this would look better I think:
| Display_Order = [ | |
| display_order = [ |
|
I think that it would worth addressing the missing support for 3D regions in a different PR (users work mainly with 2D rasters so I would prefer showing 3D params in the history pane to be optional, and besides that we also need the backward compability - should be clear for users that history records stored before the 3D support do not have 3D params shown in the pane although they might have been important for a particular history command). I plan to open the PR with 3D support soon (but need your patience - with my newborn daughter it is a slow process 🙂) and I would suggest going back to this PR afterwards. Do you agree with that? |
4 participants
This PR adds the feature referred in #5283
Here are the outputs:
MAPSET/windowsMAPSET/windowsAdded two new methods to support region name display:
_get_saved_region_name(): Searches MAPSET/windows for matching saved regions_compare_regions(): Compares regions with epsilon tolerance for floating-point precisionThese methods are separate from the existing region match logic to avoid modifying
working functionality and keep changes minimal and isolated.
Let me know if this needs any changes.