Skip to content

wxGUI/gmodeler: Correct variable replacement in model_convert.py #7241

New issue

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

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gureckyfrantisek wants to merge 10 commits into
base: main
Choose a base branch
from
Open

wxGUI/gmodeler: Correct variable replacement in model_convert.py #7241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ae826ab
fix: Correct variable replacement
gureckyfrantisek Mar 30, 2026
8f67aa1
Merge branch 'main' into issue-7212
echoix Apr 15, 2026
4c70abc
wxGUI/gmodeler: backward compatibility for curly braces
gureckyfrantisek Apr 15, 2026
5a1f52d
wxGUI/gmodeler: parsing without curly braces fix
gureckyfrantisek Apr 15, 2026
5b128c6
Merge branch 'main' into issue-7212
gureckyfrantisek Apr 29, 2026
20ccd1c
regex: Allow only both or none curly braces
gureckyfrantisek Apr 29, 2026
bb54a2a
Merge branch 'main' into issue-7212
gureckyfrantisek May 7, 2026
952c3dd
fix: Proper map name from model
gureckyfrantisek May 15, 2026
d7eb782
fix: Map variable replacement after Python
gureckyfrantisek May 15, 2026
d9c9070
fix: Map variable replacement on Display change
gureckyfrantisek May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions gui/wxpython/gmodeler/canvas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,19 +482,29 @@ def OnHasDisplay(self, event):
shape.SetHasDisplay(event.IsChecked())
self.frame.canvas.Refresh()

model = self.frame.GetModel()
run_params = getattr(model, "_runParams", None)
resolved = {}
if run_params and "variables" in run_params:
for p in run_params["variables"]["params"]:
name = p.get("name", "")
value = p.get("value", "")
if name and value:
resolved[name] = value

try:
if event.IsChecked():
# add map layer to display
self.frame._giface.GetLayerList().AddLayer(
ltype=shape.GetPrompt(),
name=shape.GetValue(),
name=shape.GetResolvedValue(resolved),
checked=True,
cmd=shape.GetDisplayCmd(),
cmd=shape.GetDisplayCmd(resolved),
)
else:
# remove map layer(s) from display
layers = self.frame._giface.GetLayerList().GetLayersByName(
shape.GetValue()
shape.GetResolvedValue(resolved)
)
for layer in layers:
self.frame._giface.GetLayerList().DeleteLayer(layer)
Expand Down
24 changes: 13 additions & 11 deletions gui/wxpython/gmodeler/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,8 @@ def Validate(self):
sval = pattern.search(value)
if not sval:
continue
var = sval.group(2).strip()[2:-1] # strip '%{...}'
s = sval.group(2).strip()
var = s[2:-1] if s.startswith("%{") else s[1:] # strip curly braces only if present
found = False
for v in variables:
if var.startswith(v):
Expand Down Expand Up @@ -539,7 +540,8 @@ def _substituteFile(self, item, params=None, checkOnly=False):
write = False
variables = self.GetVariables()
for variable in variables:
pattern = re.compile("%{" + variable + "}")
# curly braces are optional
pattern = re.compile(r"%(?:\{" + variable + r"\}|" + variable + r")")
value = ""
if params and "variables" in params:
for p in params["variables"]["params"]:
Expand All @@ -560,7 +562,8 @@ def _substituteFile(self, item, params=None, checkOnly=False):
pattern = re.compile(r"(.*)(%\{.+})(.*)")
sval = pattern.search(data)
if sval:
var = sval.group(2).strip()[2:-1] # ignore '%{...}'
s = sval.group(2).strip()
var = s[2:-1] if s.startswith("%{") else s[1:] # strip curly braces only if present
cmd = item.GetLog(string=False)[0]
errList.append(cmd + ": " + _("undefined variable '%s'") % var)

Expand Down Expand Up @@ -690,7 +693,8 @@ def Run(self, log, onDone, parent=None):
# substitute variables in condition
variables = self.GetVariables()
for variable in variables:
pattern = re.compile("%{" + variable + "}")
# curly braces are optional
pattern = re.compile(r"%(?:\{" + variable + r"\}|" + variable + r")")
if not pattern.search(cond):
continue
value = ""
Expand All @@ -713,7 +717,8 @@ def Run(self, log, onDone, parent=None):
# split condition
# TODO: this part needs some better solution
condVar, condText = (x.strip() for x in re.split(r"\s* in \s*", cond))
pattern = re.compile("%{" + condVar + "}")
# curly braces are optional
pattern = re.compile(r"%(?:\{" + condVar + r"\}|" + condVar + r")")
# for vars()[condVar] in eval(condText): ?
vlist = []
if condText[0] == "`" and condText[-1] == "`":
Expand Down Expand Up @@ -742,12 +747,9 @@ def Run(self, log, onDone, parent=None):

if delInterData:
self.DeleteIntermediateData(log)

# discard values
if params:
for item in params.values():
for p in item["params"]:
p["value"] = ""

# store run params
self._runParams = params

def DeleteIntermediateData(self, log):
"""Delete intermediate data"""
Expand Down
10 changes: 6 additions & 4 deletions gui/wxpython/gmodeler/model_convert.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ def _writeItem(self, item, ignoreBlock=True, variables={}):
# substitute condition
cond = item.GetLabel()
for variable in self.model.GetVariables():
pattern = re.compile("%{" + variable + "}")
# curly braces are optional
pattern = re.compile(r"%(?:\{" + variable + r"\}|" + variable + r")")
if pattern.search(cond):
value = variables[variable].get("value", "")
if variables[variable].get("type", "string") == "string":
Expand Down Expand Up @@ -969,14 +970,15 @@ def _substitutePythonParamValue(
# check for variables
formattedVar = False
for var in variables["vars"]:
pattern = re.compile("%{" + var + "}")
found = pattern.search(value)
# curly braces are optional
Copy link
Copy Markdown
Member

@landam landam Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curly braces enclosing variables were introduced in version 8.5. I assumed this change was meant to ensure backward compatibility, which would be nice. I tested to load model without braces (attached in the issue), the conversion to Python works. But attempt to run the model (directly from the modeler, not converted Python code) fails - no variables are substituted.

p.in.pdal -e -o --overwrite input=%path/dmp1g/%tile.laz output=%tile_dmp method=mean type=FCELL zscale=1.0 iscale=1.0 resolution=%resolution dimension=z
...

Backward compatibility wasn’t explicitly mentioned in the issue. I think it’s fine to keep this change in the PR, but in that case the model execution should be fixed as well.

Copy link
Copy Markdown
Author

@gureckyfrantisek gureckyfrantisek Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the latest commits I changed every re.compile in the gmodeler folder have optional curly braces.

I managed to both run the model directly and have the values parametrized in the generated python code, but I had a leftover empty raster in my map at the end of the process:

Failed to run command 'd.rast map=%tile_chm'. Details:

GRASS_INFO_ERROR(70582,1): Raster map <%tile_chm> not found
GRASS_INFO_END(70582,1)
Failed to run command 'd.rast map=%tile_chm'. Details:

GRASS_INFO_ERROR(70665,1): Raster map <%tile_chm> not found
GRASS_INFO_END(70665,1)

I couldn't see, where the d.rast function is called, it's not in the model or the python code.

It doesn't break it, so I didn't look for a solution yet.

Copy link
Copy Markdown
Member

@landam landam May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gureckyfrantisek It should be relatively easy to fix see https://github.com/OSGeo/grass/blob/main/gui/wxpython/gmodeler/model_items.py#L692

Copy link
Copy Markdown
Member

@landam landam May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to fix in this PR.

pattern = re.compile(r"%(?:\{" + var + r"\}|" + var + r")")
found = pattern.search(parameterizedValue)
if found:
foundVar = True
if found.end() != len(value):
formattedVar = True
parameterizedValue = pattern.sub(
"{options['" + var + "']}", value
"{options['" + var + "']}", parameterizedValue
)
else:
parameterizedValue = f'options["{var}"]'
Expand Down
31 changes: 27 additions & 4 deletions gui/wxpython/gmodeler/model_items.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,8 @@ def GetLog(self, string=True, substitute=None):

# order variables by length
for variable in sorted(variables, key=len, reverse=True):
pattern = re.compile("%{" + variable + "}")
# curly braces are optional
pattern = re.compile(r"%(?:\{" + variable + r"\}|" + variable + r")")
value = ""
if substitute and "variables" in substitute:
for p in substitute["variables"]["params"]:
Expand Down Expand Up @@ -678,8 +679,30 @@ def Update(self):
self._setPen()
self.SetLabel()

def GetDisplayCmd(self):
"""Get display command as list"""
def GetResolvedValue(self, resolved=None):
"""Get value with model substituted variables

:param resolved: dict mapping variable name to resolved value,
or None to return the raw value
"""
if not resolved:
return self.value
value = self.value

# find the variable in resolved
for variable, var_value in resolved.items():
pattern = re.compile(r"%(?:\{" + variable + r"\}|" + variable + r")")
value = pattern.sub(var_value, value)

# return substituted value
return value

def GetDisplayCmd(self, resolved=None):
"""Get display command as list

:param resolved: dict mapping variable name to resolved value,
or None to return the raw value
"""
cmd = []
if self.prompt == "raster":
cmd.append("d.rast")
Expand All @@ -689,7 +712,7 @@ def GetDisplayCmd(self):
msg = "Unsupported display prompt: {}".format(self.prompt)
raise GException(msg)

cmd.append("map=" + self.value)
cmd.append("map=" + self.GetResolvedValue(resolved))

return cmd

Expand Down
53 changes: 50 additions & 3 deletions gui/wxpython/gmodeler/panels.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,24 +408,41 @@ def OnModelDone(self, event):
# delete intermediate data
self._deleteIntermediateData()

# store resolved variables
run_params = getattr(self.model, "_runParams", None)
resolved = {}
if run_params and "variables" in run_params:
for p in run_params["variables"]["params"]:
name = p.get("name", "")
value = p.get("value", "")
if name and value:
resolved[name] = value

# display data if required
for data in self.model.GetData():
if not data.HasDisplay():
continue

# remove existing map layers first
layers = self._giface.GetLayerList().GetLayersByName(data.GetValue())
layers = self._giface.GetLayerList().GetLayersByName(data.GetResolvedValue(resolved))
if layers:
for layer in layers:
self._giface.GetLayerList().DeleteLayer(layer)

# add new map layer
self._giface.GetLayerList().AddLayer(
ltype=data.GetPrompt(),
name=data.GetValue(),
name=data.GetResolvedValue(resolved),
checked=True,
cmd=data.GetDisplayCmd(),
cmd=data.GetDisplayCmd(resolved),
)

# discard values
if run_params:
for item in run_params.values():
for p in item["params"]:
p["value"] = ""
del self.model._runParams

def _switchPageHandler(self, event, notification):
self._switchPage(notification=notification)
Expand Down Expand Up @@ -1801,6 +1818,36 @@ def OnDone(self, event):
try_remove(self.filename)
self.filename = None

# store resolved variables
model = self.parent.GetModel()
run_params = getattr(model, "_runParams", None)
resolved = {}
if run_params and "variables" in run_params:
for p in run_params["variables"]["params"]:
name = p.get("name", "")
value = p.get("value", "")
if name and value:
resolved[name] = value

# display data if required
for data in model.GetData():
if not data.HasDisplay():
continue

# remove existing map layers first
layers = self.parent._giface.GetLayerList().GetLayersByName(data.GetResolvedValue(resolved))
if layers:
for layer in layers:
self.parent._giface.GetLayerList().DeleteLayer(layer)

# add new map layer
self.parent._giface.GetLayerList().AddLayer(
ltype=data.GetPrompt(),
name=data.GetResolvedValue(resolved),
checked=True,
cmd=data.GetDisplayCmd(resolved),
)

def OnChangeScriptType(self, event):
new_script_type = self.script_type_box.GetStringSelection()

Expand Down