Skip to content
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.

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

style: Fix missing-f-string-syntax (RUF027) #5031

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

Conversation

echoix
Copy link
Member

@echoix echoix commented Feb 2, 2025

RUF027 Possible f-string without an f prefix

Ruff rule: https://docs.astral.sh/ruff/rules/missing-f-string-syntax/

@github-actions github-actions bot added vector Related to vector data processing raster Related to raster data processing Python Related code is in Python database Related to database management libraries module labels Feb 2, 2025
Comment on lines 59 to 69
if red:
expressions.append("%s = r#${input}" % red)
expressions.append("%s = r#${input}" % red) # noqa: RUF027
maps.append(red)
if green:
expressions.append("%s = g#${input}" % green)
expressions.append("%s = g#${input}" % green) # noqa: RUF027
maps.append(green)
if blue:
expressions.append("%s = b#${input}" % blue)
expressions.append("%s = b#${input}" % blue) # noqa: RUF027
maps.append(blue)
expr = ";".join(expressions)
gs.mapcalc(expr, input=input)
Copy link
Member

@wenzeslaus wenzeslaus Feb 3, 2025

Choose a reason for hiding this comment

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

This brings up the larger issue of r.mapcalc syntax in Python. String templates (string.Template) were adopted to be used internally, but it was never mandatory (it can't be) and it was before format function and f-strings and their appeal was closeness to shell/Bash substitution. Having theses RUF ignores here is not nice. It could just switch to f-strings. I'm not sure what to do here in this PR specifically.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can input be safely substituted here without passing the "input" to mapcalc? Otherwise we can keep it as I changed it here, that is to not use an f-string because of the conflict. It's not that bad to say no to a syntax

Copy link
Member

Choose a reason for hiding this comment

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

I think it us the same, but on a second thought, this PR is not the place to change the practice.

@echoix echoix enabled auto-merge (squash) February 4, 2025 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Related to database management libraries module Python Related code is in Python raster Related to raster data processing vector Related to vector data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants