-
-
Notifications
You must be signed in to change notification settings - Fork 364
i.in.spotvgt: bug fixes #6179
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
base: main
Are you sure you want to change the base?
i.in.spotvgt: bug fixes #6179
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a question/suggestion for the rules block change, and I'm not sure of commenting out the temporary project and its effects. Please someone evaluate this.
scripts/i.in.spotvgt/i.in.spotvgt.py
Outdated
rules = """8 50 50 50 | ||
11 70 70 70 | ||
12 90 90 90 | ||
60 grey | ||
155 blue | ||
232 violet | ||
235 red | ||
236 brown | ||
248 orange | ||
251 yellow | ||
252 green | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to verify that the string doesn't have leading whitespace?
If you start by changing to something like the "bad" of this rule: https://docs.astral.sh/ruff/rules/static-join-to-f-string/, so using str.join(), you should end up with ruff making a suggestion for the correct fix.
So, something like removing the list brackets:
rules = """8 50 50 50 | |
11 70 70 70 | |
12 90 90 90 | |
60 grey | |
155 blue | |
232 violet | |
235 red | |
236 brown | |
248 orange | |
251 yellow | |
252 green | |
""" | |
rules = "\n".join([ | |
"8 50 50 50", | |
"11 70 70 70", | |
"12 90 90 90", | |
"60 grey", | |
"155 blue", | |
"232 violet", | |
"235 red", | |
"236 brown", | |
"248 orange", | |
"251 yellow", | |
"252 green", | |
] | |
) |
And then applying the fixes unblocked. Having this inside a list was probably preventing rules to apply at it wouldn't have meant the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially tried using "\n".join([...])
, but Ruff’s FLY002
check flagged it. To avoid leading whitespace issues and still satisfy Ruff, I will switch to the following:
rules = (
"8 50 50 50\n"
"11 70 70 70\n"
"12 90 90 90\n"
"60 grey\n"
"155 blue\n"
"232 violet\n"
"235 red\n"
"236 brown\n"
"248 orange\n"
"251 yellow\n"
"252 green\n"
)
d5b978c
to
4f78a2b
Compare
This PR addresses the four issues in the
i.in.spotvgt
module that previously led to incorrect imports or completely null raster outputs as highlighted in #6175.Summary of Changes:
if f < 2:
caused type error due to comparison of list directly, it has been correctly fixed asif len(f) < 2:
qname = spotname.replace("NDV", "SM")
resulted in missing file error due to missing extension. This has been fixed by changing it toqname = spotname.replace("NDV", "SM") + ".HDF"
raised
TypeError
asstdin
can only accept string or bytes. This has been fixed by modifying it to:gs.use_temp_region()
caused the output map to be null, since the temporary region prevented g.rename and r.mapcalc results from being preserved in main mapset. This line has been commented out.This PR closes #6175.