Checks: Fix E722 `do not use bare except` #3748

mshukuno · 2024-05-28T23:26:55Z

The entire fix involves converting except: into except Exception:. Please let me know if specific exceptions are needed in certain conditions.

wenzeslaus

Great. except Exception is far better than bare except which catches even syntax errors, but later we will aim at Pylint and that asks for something more specific than Exception.

It seems easier to see the exceptions here in this PR than going to Pylint later on, so fixing as much as possible now would be good.

Nevertheless, I would aim at the clear cases in this PR. I tried to mark couple of cases which are clear and will be repeated over several files. Ultimately, you need to look at every case. If you need to read more than 10 lines of code to understand the context, leaving Exception is probably the right choice.

wenzeslaus · 2024-05-29T00:41:24Z

scripts/r.in.wms/wms_gdal_drv.py

 try:
    from osgeo import gdal
-except:
+except Exception:


Cases like this are import error.

except ImportError:

utils/gitlog2changelog.py

wenzeslaus · 2024-05-29T00:51:25Z

scripts/r.in.wms/srs.py

            try:
                self.code = int(values[-1])
-            except:
+            except Exception:


This is a case where code context helps: values clearly has at least one value, so the indexing will succeed here. What can fail is the conversion to an integer.

except ValueError:

wenzeslaus · 2024-05-29T00:54:53Z

scripts/r.in.srtm/r.in.srtm.py

    try:
        grass.run_command("r.in.gdal", input=bilfile, out=tileout)
-    except:
+    except Exception:


The provided error message is trivial. This will be better handled by removing try-except and simply asking run_command to issue the fatal error with errors="fatal":

run_command(..., errors="fatal")

What about grass.read_command? It appears that it works with the "errors" parameter. A question would be the status of the errors. For example, gui/wxpython/core/utils.py

def _getGDALFormats(): """Get dictionary of available GDAL drivers""" try: ret = grass.read_command("r.in.gdal", quiet=True, flags="f") except Exception: ret = None return _parseFormats(ret), _parseFormats(ret, writableOnly=True) def _getOGRFormats(): """Get dictionary of available OGR drivers""" try: ret = grass.read_command("v.in.ogr", quiet=True, flags="f") except Exception: ret = None return _parseFormats(ret), _parseFormats(ret, writableOnly=True)

That should be grass.exceptions.CalledModuleError:
https://grass.osgeo.org/grass84/manuals/libpython/grass.exceptions.html#grass.exceptions.CalledModuleError

Note: The difference here is gui/wxpython/ versus a scripts/.

…E722

…flake8-E722

mshukuno

I'm reading the code between try and except and finding possible errors, so you may want to disregard some of the errors I listed in except. As a side note, I used code assistant to find out some of the most common exceptions, and then evaluated whether they might occur in the given code.

Third-party library exceptions (e.g. wxPython) and nested internal function calls related exceptions are difficult to figure out. In such a situation, broad exceptions are useful, so I kept them as they are, but please let me know if you have any suggestions for specific exceptions.

mshukuno · 2024-05-31T18:57:48Z

gui/wxpython/core/gconsole.py

@@ -676,7 +676,7 @@ def RunCmd(
            if len(command) == 1 and not skipInterface:
                try:
                    task = gtask.parse_interface(command[0])
-                except:
+                except Exception:


I wanted to note any changes I made along the way.
parse_interface

from grass.exceptions import ScriptError except ScriptError:

mshukuno · 2024-05-31T19:57:14Z

gui/wxpython/core/render.py

@@ -1242,7 +1242,7 @@ def SetRegion(self, windres=False, windres3=False):

            return grass_region

-        except:
+        except Exception:


except (KeyError, TypeError):

mshukuno · 2024-05-31T23:39:02Z

gui/wxpython/core/settings.py

@@ -110,7 +110,7 @@ def _generateLocale(self):
            self.locs.sort()
            # Add a default choice to not override system locale
            self.locs.insert(0, "system")
-        except:
+        except Exception:


except (KeyError, FileNotFoundError, PermissionError, NotADirectoryError, OSError):

KeyError: This would occur if the environment variable "GISBASE" is not set.

FileNotFoundError: This would be raised by os.listdir() if the directory specified does not exist. In this case, if the "locale" directory does not exist within the directory specified by "GISBASE".

PermissionError: This would be raised by os.listdir() if the user does not have the necessary permissions to read the contents of the directory.

NotADirectoryError: This would be raised by os.listdir() if the path exists but is not a directory.

OSError: This would be raised by os.listdir() for reasons like insufficient system resources or other OS-related issues.

mshukuno · 2024-05-31T23:39:11Z

gui/wxpython/core/settings.py

@@ -992,7 +992,7 @@ def SaveToFile(self, settings=None):
        if not os.path.exists(dirPath):
            try:
                os.mkdir(dirPath)
-            except:
+            except Exception:


except OSError:

gui/wxpython/core/toolboxes.py

mshukuno · 2024-06-03T17:22:36Z

scripts/v.unpack/v.unpack.py

@@ -77,7 +77,7 @@ def main():
    tar = tarfile.TarFile.open(name=input_base, mode="r")
    try:
        data_name = tar.getnames()[0]
-    except:
+    except Exception:


except (AttributeError, IndexError, tarfile.ReadError):

AttributeError: This exception is raised if the tar object does not have a getnames method. This could occur if tar is not a TarFile object.

IndexError: This exception is raised during the tar.getnames()[0] call if the result of tar.getnames() does not have at least one element. This could occur if the tar file is empty.

tarfile.ReadError: This exception is raised if the tar file cannot be read, which might happen if the file is not a tar file or is corrupted.

scripts/v.what.strds/v.what.strds.py

utils/gitlog2changelog.py

mshukuno · 2024-06-03T17:25:39Z

utils/gitlog2changelog.py

@@ -66,7 +66,7 @@
            author = authorList[1]
            author = author[0 : len(author) - 1]
            authorFound = True
-        except:
+        except Exception:


except IndexError:

mshukuno · 2024-06-03T17:26:02Z

utils/gitlog2changelog.py

@@ -76,7 +76,7 @@
            date = dateList[1]
            date = date[0 : len(date) - 1]
            dateFound = True
-        except:
+        except Exception:


except (IndexError, TypeError):

…flake8-E722

wenzeslaus · 2024-06-04T03:03:21Z

python/grass/temporal/spatial_topology_dataset_connector.py

@@ -118,31 +118,31 @@ def get_number_of_spatial_relations(self):
        relations = {}
        try:
            relations["equivalent"] = len(self._spatial_topology["EQUIVALENT"])
-        except:
+        except (KeyError, TypeError):


The class, e.g., def append_equivalent(self, map):, always creates the dict item with an empty list, so the len call should always succeed, so there should be no TypeError, or if there is, it is programmer's fault and traceback is the desired behavior.

Keeping except Exception for now?

I meant the opposite. If you analyze the code, it should reveal that of the dict item is there, it will be an empty list. So, no TypeError. The attribute seems to be present. (Please check.) This would leave KeyError as what the original author intended to catch here.

scripts/r.in.srtm/r.in.srtm.py

wenzeslaus · 2024-06-04T03:12:40Z

scripts/v.unpack/v.unpack.py

+    except (AttributeError, IndexError, tarfile.ReadError):
        grass.fatal(_("Pack file unreadable"))


Same as the other extract code: Report the error to the user.

I am guessing you are refering to scripts/r.in.srtm/r.in.srtm.py and changed to as following.

except (AttributeError, IndexError, tarfile.ReadError) as error: grass.fatal(_("Pack file unreadable: {error}").format(error=error))

Did you push the change?

wenzeslaus · 2024-06-04T03:13:43Z

utils/gitlog2changelog.py

@@ -76,7 +76,7 @@
            date = dateList[1]
            date = date[0 : len(date) - 1]
            dateFound = True
-        except:
+        except (IndexError, TypeError):


Why TypeError?

TypeError: This will occur if dateList is not a list or if dateList[1] is not a string. This can happen if the input to this code snippet is not as expected.

dateList and dateList[1] are a result of the re.split function call on line 74 which returns a list of strings according to the documentation.

wenzeslaus

I left comments for several changes. Some of them hopefully apply generally, too.

github-actions · 2024-06-05T23:03:04Z

scripts/r.in.srtm/r.in.srtm.py

@@ -230,8 +230,8 @@ def main():
        try:
            zf = zfile.ZipFile(zipfile)
            zf.extractall()
-        except:
-            grass.fatal(_("Unable to unzip file."))
+        except (FileNotFoundError, PermissionError, OSError, zipfile.BadZipFile) as error:


[black] _{reported by reviewdog 🐶}

Suggested change

except (FileNotFoundError, PermissionError, OSError, zipfile.BadZipFile) as error:

except (

FileNotFoundError,

PermissionError,

OSError,

zipfile.BadZipFile,

) as error:

wenzeslaus · 2024-06-06T16:06:47Z

lib/init/grass.py

@@ -2213,7 +2213,7 @@ def print_params(params):
            try:
                revision = linerev.split(" ")[1]
                sys.stdout.write("%s\n" % revision[1:])
-            except:
+            except (IndexError, TypeError, AttributeError):


AttributeError - Generally, attribute error should not be caught unless there is a specific reason to do so (like dealing with different types of objects and reacting to the differences in their attributes).

TypeError - Where this would happen and is that something we want to hide or cause traceback?

TypeError happens as following.

TypeError: This exception could be raised by revision[1:] if revision is not a string or bytes-like object. The slicing operation is not supported by all data types, so if revision is not of a type that supports slicing, a TypeError would be raised.

Under which conditions revision would not be a string? The revision value is a result of the line 2214 which splits a string using str.split which returns list of the words in the string.

echoix · 2024-10-17T03:37:47Z

Fixed conflicts, maybe some changes wouldn't still be needed since some work has already been done (and those lines would revert back if I didn't do my job correctly)

Fix E722 do not use bare except

2b60b99

github-actions bot added GUI wxGUI related vector Related to vector data processing raster Related to raster data processing Python Related code is in Python translation Message translation related libraries module docs display imagery labels May 28, 2024

wenzeslaus requested changes May 29, 2024

View reviewed changes

mshukuno and others added 3 commits May 30, 2024 11:58

Merge branch 'OSGeo:main' into flake8-E722

ad41ff7

Merge branch 'main' of https://github.com/mshukuno/grass into flake8-…

b444ea6

…E722

Merge branch 'flake8-E722' of https://github.com/mshukuno/grass into …

f0bfc76

…flake8-E722

mshukuno commented Jun 3, 2024

View reviewed changes

mshukuno and others added 4 commits June 3, 2024 15:30

Merge branch 'OSGeo:main' into flake8-E722

c646724

Merge branch 'flake8-E722' of https://github.com/mshukuno/grass into …

70512e1

…flake8-E722

Fix some of broat-exception

f66788a

black

bce6eeb

wenzeslaus reviewed Jun 4, 2024

View reviewed changes

scripts/r.in.srtm/r.in.srtm.py Outdated Show resolved Hide resolved

wenzeslaus reviewed Jun 4, 2024

View reviewed changes

wenzeslaus requested changes Jun 4, 2024

View reviewed changes

Implementing suggested changes from the code review

9b3b261

github-actions bot reviewed Jun 5, 2024

View reviewed changes

wenzeslaus reviewed Jun 6, 2024

View reviewed changes

mshukuno added 2 commits June 10, 2024 13:37

Implementing suggested changes from the code review

21b4199

Remove AttributeError

0f5a689

mshukuno added 2 commits June 10, 2024 13:40

Update exceptions

cf4f4b1

Reformat with black

a568514

mshukuno force-pushed the flake8-E722 branch from 6161922 to a568514 Compare June 10, 2024 17:43

wenzeslaus mentioned this pull request Sep 21, 2024

[Bug] Fix currently ignored Flake8 warnings #1522

Open

wenzeslaus self-assigned this Sep 21, 2024

echoix added 2 commits October 17, 2024 03:33

Merge branch 'main' into flake8-E722

42358fa

style: Fix UP024 - Replace aliased errors with OSError

4636a1b

echoix removed GUI wxGUI related vector Related to vector data processing raster Related to raster data processing Python Related code is in Python translation Message translation related libraries module docs display imagery labels Oct 17, 2024

github-actions bot added GUI wxGUI related vector Related to vector data processing raster Related to raster data processing Python Related code is in Python translation Message translation related libraries module docs display imagery labels Oct 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Checks: Fix E722 `do not use bare except` #3748

Checks: Fix E722 `do not use bare except` #3748

mshukuno commented May 28, 2024

wenzeslaus left a comment

wenzeslaus May 29, 2024

mshukuno Jun 3, 2024

wenzeslaus May 29, 2024

mshukuno Jun 3, 2024

wenzeslaus May 29, 2024

mshukuno May 29, 2024

petrasovaa May 29, 2024

wenzeslaus May 30, 2024

mshukuno left a comment

mshukuno May 31, 2024

mshukuno May 31, 2024

mshukuno May 31, 2024

mshukuno May 31, 2024

mshukuno Jun 3, 2024

mshukuno Jun 3, 2024

mshukuno Jun 3, 2024

wenzeslaus Jun 4, 2024

mshukuno Jun 5, 2024

wenzeslaus Jun 7, 2024

wenzeslaus Jun 4, 2024

mshukuno Jun 5, 2024

wenzeslaus Jun 7, 2024

wenzeslaus Jun 4, 2024

mshukuno Jun 5, 2024

wenzeslaus Jun 7, 2024

wenzeslaus left a comment

github-actions bot Jun 5, 2024

wenzeslaus Jun 6, 2024

mshukuno Jun 6, 2024

wenzeslaus Jun 7, 2024

echoix commented Oct 17, 2024

		except (AttributeError, IndexError, tarfile.ReadError):
		grass.fatal(_("Pack file unreadable"))

Checks: Fix E722 do not use bare except #3748

Are you sure you want to change the base?

Checks: Fix E722 do not use bare except #3748

Conversation

mshukuno commented May 28, 2024

wenzeslaus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshukuno left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wenzeslaus left a comment

Choose a reason for hiding this comment

github-actions bot Jun 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

echoix commented Oct 17, 2024

Checks: Fix E722 `do not use bare except` #3748

Checks: Fix E722 `do not use bare except` #3748