style: scripts/v.report/v.report.py:fixed F841 local variable 'numcols' is assigned to but never used #1609
Conversation
…ssigned to but never used
… assigned to but never used
scripts/i.band/i.band.py
Outdated
| @@ -63,7 +63,7 @@ def print_map_band_reference(name, band_reader): | |||
| band_reader.print_info(shortcut, band) | |||
| else: | |||
| gs.info(_("No band reference assigned to <{}>").format(name)) | |||
| except OpenError as e: | |||
| except OpenError as e: # noqa: F841 | |||
There was a problem hiding this comment.
No need to keep these XyzException as error pieces if the error is not used (except XyzException is perfectly valid), but here, can you check the source code for what OpenError contains, i.e., if it has any more specific message which would add details to (or replace).
@landam can comment here in case he had some intentions with the captured error originally.
There was a problem hiding this comment.
Already addressed in #4241, but since this PR, this module was renamed to r.semantic.label.
scripts/i.band/i.band.py
Outdated
| @@ -95,7 +95,7 @@ def manage_map_band_reference(name, band_ref): | |||
| except GrassError as e: | |||
| gs.error(_("Unable to assign/dissociate band reference. {}").format(e)) | |||
| return 1 | |||
| except OpenError as e: | |||
| except OpenError as e: # noqa: F841 | |||
| table = options["table"] | ||
| table = options["table"] # noqa: F841 |
There was a problem hiding this comment.
The fact that it is unused looks like a bug. Can you please check the rest of the code to see if it is potentially missing anywhere (like parameter for another module or something like layer is used instead of it in an SQL expression? Alternatively, you can just leave this out for this PR.
There was a problem hiding this comment.
I looked at it and this looks like option table was never used, so I think we should remove it in a separate PR. It would sort of break backwards compatibility, but if the option was never used, I assume it would be fine.
|
Thanks @ShubhamSwati! The smaller PRs are much easier to review and perhaps we can get some PRs merged sooner while still working on others. |
|
Bumping up milestone as 8.0.1 is due in two days, while this has not been part of RC1 and there has not been activity for some time. |
|
This is not directly fixing any bug, although it is fixing and improving the code, so no need to keep it in 8.0 series. @ninsbl feel free to move milestone for PRs/issues like this without any comment. You can also simply decide based on the absence of blocker label. No blocker label? Move to next minor. |
| @@ -67,7 +67,7 @@ def main(): | |||
| layer = options["layer"] | |||
| format = options["format"] | |||
| output = options["output"] | |||
| table = options["table"] | |||
| table = options["table"] # noqa: F841 | |||
There was a problem hiding this comment.
| table = options["table"] # noqa: F841 | |
| table = options["table"] |
scripts/i.band/i.band.py
Outdated
| @@ -63,7 +63,7 @@ def print_map_band_reference(name, band_reader): | |||
| band_reader.print_info(shortcut, band) | |||
| else: | |||
| gs.info(_("No band reference assigned to <{}>").format(name)) | |||
| except OpenError as e: | |||
| except OpenError as e: # noqa: F841 | |||
There was a problem hiding this comment.
Already addressed in #4241, but since this PR, this module was renamed to r.semantic.label.
scripts/i.band/i.band.py
Outdated
| @@ -95,7 +95,7 @@ def manage_map_band_reference(name, band_ref): | |||
| except GrassError as e: | |||
| gs.error(_("Unable to assign/dissociate band reference. {}").format(e)) | |||
| return 1 | |||
| except OpenError as e: | |||
| except OpenError as e: # noqa: F841 | |||
|
I managed to solve the conflicts of the renamed files and updated 3 years of commits to the PR. Some issues have been fixed since |
echoix
changed the title
|
@arohanajit Do you want to take a look and try to review this? With all the experience you've had until now, you could also try the reverse role ;) It's an old PR, that when solving conflicts, there's not much left. You might want to take a look that the correct exclusions are used, as the flake8 config file used before my merge today was in another folder, and doesn't exist anymore. |
This one isn't fixed either. And I don't think this PR is actually doing anything. This error is also listed in the current |
…ssigned to but never used