python: Fix E501 in r.import and clean up .flake8 configuration · Pull Request #7881 · OSGeo/grass

ghost · 2026-09-03T14:23:45Z

This is a clean rewrite of #7880. Fixed E501 warnings in r.import.py and properly split long metadata lines using multi-line # % blocks to ensure compatibility with g.parser, as suggested by @echoix.

echoix · 2026-09-03T14:31:29Z

Todo: check if multiline descriptions are allowed, and picked up by translation code, and by the docs generation, new markdown and maybe the old path too

I'm not so sure how to check that.

github-actions · 2026-09-03T16:48:40Z


 # %module
-# % description: Imports raster data into a GRASS raster map using GDAL library and reprojects on the fly.
+# % description: Imports raster data into a GRASS raster map using GDAL library 


[ruff] _{reported by reviewdog 🐶}

Suggested change

# % description: Imports raster data into a GRASS raster map using GDAL library

# % description: Imports raster data into a GRASS raster map using GDAL library

github-actions · 2026-09-03T16:48:40Z

+        gs.run_command(
+            "r.in.gdal", input=GDALdatasource, flags="j", quiet=True
+        )


[ruff] _{reported by reviewdog 🐶}

Suggested change

gs.run_command(

"r.in.gdal", input=GDALdatasource, flags="j", quiet=True

)

gs.run_command("r.in.gdal", input=GDALdatasource, flags="j", quiet=True)

github-actions · 2026-09-03T16:48:40Z

+    if (
+        options["resolution_value"]
+    ):


[ruff] _{reported by reviewdog 🐶}

Suggested change

if (

options["resolution_value"]

):

if options["resolution_value"]:

github-actions · 2026-09-03T16:48:40Z

+                _(
+                    "To set custom resolution value, select"
+                    "'value' in resolution option"
+                )


[ruff] _{reported by reviewdog 🐶}

Suggested change

_(

"To set custom resolution value, select"

"'value' in resolution option"

)

_("To set custom resolution value, select'value' in resolution option")

github-actions · 2026-09-03T16:48:40Z

+            gs.fatal(
+                _("Resolution value can't be smaller than 0")
+            )


[ruff] _{reported by reviewdog 🐶}

Suggested change

gs.fatal(

_("Resolution value can't be smaller than 0")

)

gs.fatal(_("Resolution value can't be smaller than 0"))

github-actions · 2026-09-03T16:48:41Z

+            _(
+                "Estimated target resolution for input band <{out}>: {res}"
+            ).format(


[ruff] _{reported by reviewdog 🐶}

Suggested change

_(

"Estimated target resolution for input band <{out}>: {res}"

).format(

_("Estimated target resolution for input band <{out}>: {res}").format(

github-actions · 2026-09-03T16:48:41Z

+            env["GRASS_REGION"] = gs.region_env(
+                n=n, s=s, e=e, w=w
+            )


[ruff] _{reported by reviewdog 🐶}

Suggested change

env["GRASS_REGION"] = gs.region_env(

n=n, s=s, e=e, w=w

)

env["GRASS_REGION"] = gs.region_env(n=n, s=s, e=e, w=w)

github-actions · 2026-09-03T16:48:41Z

+                _(
+                    "Using given resolution for input band <{out}>: {res}"
+                ).format(


[ruff] _{reported by reviewdog 🐶}

Suggested change

_(

"Using given resolution for input band <{out}>: {res}"

).format(

_("Using given resolution for input band <{out}>: {res}").format(

github-actions · 2026-09-03T16:48:41Z

+            env["GRASS_REGION"] = gs.region_env(
+                res=res, flags="a", env=env
+            )


[ruff] _{reported by reviewdog 🐶}

Suggested change

env["GRASS_REGION"] = gs.region_env(

res=res, flags="a", env=env

)

env["GRASS_REGION"] = gs.region_env(res=res, flags="a", env=env)

github-actions · 2026-09-03T16:48:41Z

+                ).format(
+                    out=outfile,
+                    ns=curr_reg["nsres"],
+                    ew=curr_reg["ewres"]
+                )


[ruff] _{reported by reviewdog 🐶}

Suggested change

).format(

out=outfile,

ns=curr_reg["nsres"],

ew=curr_reg["ewres"]

)

).format(out=outfile, ns=curr_reg["nsres"], ew=curr_reg["ewres"])

github-actions · 2026-09-03T16:51:16Z

+                n = gs.float_or_dms(
+                    srcregion["n"][:-1]
+                ) * (-1 if srcregion["n"][-1] == "S" else 1)
+


[pre-commit] _{reported by reviewdog 🐶}

Suggested change

github-actions · 2026-09-03T16:51:16Z

+                s = gs.float_or_dms(
+                    srcregion["s"][:-1]
+                ) * (-1 if srcregion["s"][-1] == "S" else 1)
+


[pre-commit] _{reported by reviewdog 🐶}

Suggested change

github-actions · 2026-09-03T16:51:16Z

+                e = gs.float_or_dms(
+                    srcregion["e"][:-1]
+                ) * (-1 if srcregion["e"][-1] == "W" else 1)
+


[pre-commit] _{reported by reviewdog 🐶}

Suggested change

echoix · 2026-09-03T21:56:49Z

Here is an AI review of this PR, with the analysis and test results behind the decision to close it: the rewrapped # % lines silently break r.import's interface definition, and the E501 warnings they target are already ignored repo-wide.

AI review: analysis and test results

Why the rewrap breaks the tool

g.parser reads the # % block one line at a time (general/g.parser/main.c, the for (ctx.line = 1;; ctx.line++) loop): each line is split on its first : and dispatched into the state machine immediately. There is no continuation syntax and no buffering, so a wrapped value is truncated at the wrap point and the continuation line is dispatched as an unknown key.

Test

GRASS was built from this branch and g.parser run against the PR's file and against main's:

grass --exec g.parser -s scripts/r.import/r.import.py --interface-description

On the PR's version, stderr:

Unknown module parameter "and reprojects on the fly." at line 18
Unknown option parameter "bicubic;bicubic interpolation;lanczos;lanczos filter;" at line 50
Unknown option parameter "bilinear_f;bilinear interpolation with fallback;bicubic_f;" at line 51
Unknown option parameter "bicubic interpolation with fallback;lanczos_f;lanczos filter with fallback" at line 52
Unknown option parameter "user-specified resolution;region;current region resolution" at line 76
Unknown option parameter "resolution=value)" at line 85
Unknown flag parameter "(90N,S; 180E,W)" at line 109
Unknown flag parameter "system (CRS) as the current project" at line 115

On main's version, stderr is empty. Diffing the two --interface-description XML outputs (< = main, > = this PR):

- Imports raster data into a GRASS raster map using GDAL library and reprojects on the fly.
+ Imports raster data into a GRASS raster map using GDAL library

- <description>bicubic interpolation</description>
- <description>lanczos filter</description>
- <description>bilinear interpolation with fallback</description>
- <description>bicubic interpolation with fallback</description>
- <description>lanczos filter with fallback</description>

- Resolution of output raster map (use with option resolution=value)
+ Resolution of output raster map (use with option

- Force Lat/Lon maps to fit into geographic coordinates (90N,S; 180E,W)
+ Force Lat/Lon maps to fit into geographic coordinates

- Assume that the dataset has the same coordinate reference system (CRS) as the current project
+ Assume that the dataset has the same coordinate reference

The value entry of resolution also ends up with an empty <description></description> (the dangling trailing ;), and region loses its description entirely. Five of the seven resample methods lose their per-value descriptions.

This is a user-visible regression in --help, the generated r.import.md/.html, the GUI dialog, and the translation catalog (g.parser -t extracts the truncated strings), plus eight error lines on stderr on every run.

Other issues in the diff

scripts/r.import/r.import.py:186 - implicit string concatenation drops a space: the message renders as select'value' in resolution option.
scripts/r.import/r.import.py:199 - message text changed from "change to 'estimated' resolution" to "change 'estimated' resolution", which also invalidates the existing translations.
New trailing whitespace on line 17 and whitespace-only lines 394, 398, 402 - the trailing-whitespace pre-commit hook fails.
Several hand-wraps are not ruff-format compatible (e.g. if (\n options["resolution_value"]\n):, the split gs.run_command calls); the ruff-format hook will undo them, and none of those lines were over 88 characters to begin with.
.flake8 - the removed E501 per-file exceptions are for v.unpack.py and v.import.py, which this PR does not touch and which still have over-length lines. r.import.py, the file actually edited, was never in that list.

About the E501 motivation

E501 is in the global ignore list in .flake8 and in ruff's ignore list, so these lines are not being flagged by anything. The scripts/* entries in the per-file exception list exist precisely because the # % interface block has to stay one key per line regardless of length.

Could `g.parser` support continuation?

It could, but it is not a small change, and it does not look worth it:

It needs an explicit marker. A trailing \ is the only safe choice - "no colon means continuation" cannot work, since # %module, # %end and # %option G_OPT_R_INPUT are colon-less by design. No # % line in the repo currently ends in \, so that syntax is free.
The read loop in main.c would need an accumulator and a growable buffer, joining with no separator (a forced single space would inject leading spaces into descriptions: values, which parser.c splits on ; verbatim), and would need to report the first line of a continued group in the Unknown ... at line %d messages.
g.parser has no test suite today, so this would need one.
Most importantly, any script using the new syntax would silently truncate on older GRASS - the exact breakage above, with no error exit. That is a hard constraint for addons and for backports, for no benefit beyond shorter lines that nothing is complaining about.

bryzgin and others added 2 commits September 3, 2026 21:20

python: Fix E501 in r.import and clean up .flake8 configuration

c1ef4bc

Merge branch 'main' into fix-flake8-warnings

d14c20b

github-actions Bot added CI Continuous integration raster Related to raster data processing Python Related code is in Python module labels Sep 3, 2026

github-actions Bot reviewed Sep 3, 2026

View reviewed changes

echoix closed this Sep 3, 2026

echoix mentioned this pull request Sep 6, 2026

wxGUI: fix E501 in seven launchers #7904

Closed

	# % description: Imports raster data into a GRASS raster map using GDAL library
	# % description: Imports raster data into a GRASS raster map using GDAL library

Conversation

ghost commented Sep 3, 2026

Uh oh!

echoix commented Sep 3, 2026

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

echoix commented Sep 3, 2026

Why the rewrap breaks the tool

Test

Other issues in the diff

About the E501 motivation

Could g.parser support continuation?

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Could `g.parser` support continuation?