fix: replace 9 bare except clauses with except Exception#2772
Conversation
Changed across 6 files: timelapse.py, map_widgets.py, conversion.py, coreutils.py, common.py, geemap.py
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces bare except clauses with except Exception across multiple modules, including common.py, conversion.py, coreutils.py, geemap.py, map_widgets.py, and timelapse.py. The reviewer recommends further refining these handlers by catching specific exceptions—such as KeyError, ValueError, IndexError, TypeError, and ImportError—to avoid masking unrelated logic errors and to improve the precision of error handling.
| percentile_2 = min([stats[s]["percentile_2"] for s in stats]) | ||
| percentile_98 = max([stats[s]["percentile_98"] for s in stats]) | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
The try block performs dictionary lookups and calls min()/max() on list comprehensions. These operations can raise KeyError (if a key is missing) or ValueError (if the list is empty). Catching these specific exceptions is preferred over the broad Exception class to avoid masking unrelated logic errors.
except (KeyError, ValueError):| if (line.replace(" ", ""))[line.find("!") + 1] != "=": | ||
| line = line.replace("!", "not ") | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
The string indexing logic [line.find("!") + 1] is prone to an IndexError if the '!' character is at the end of the string or if the resulting string is empty. It is better to catch IndexError specifically rather than the broad Exception class.
except IndexError:| try: | ||
| toolbar_button = ipywidgets.ToggleButton(**widget_args) | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
The failure here is likely due to an unsupported keyword argument in ipywidgets.ToggleButton (such as layout in older versions), which raises a TypeError. Catching TypeError specifically is more precise for this version-compatibility check.
except TypeError:| try: | ||
| size = int(font_size.replace("pt", "")) | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
Since the try block specifically targets a potential failure in int() conversion, catching ValueError is more precise than catching the base Exception class. This is especially relevant here as you are re-raising a ValueError with a custom message.
except ValueError:| try: | ||
| from PIL import Image, ImageDraw, ImageFont, ImageSequence | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
When handling optional dependency imports, it is best practice to catch ImportError specifically. Catching the broad Exception class can mask other unexpected errors that might occur during module initialization.
| except Exception: | |
| except ImportError: |
There was a problem hiding this comment.
Pull request overview
This PR improves exception-handling hygiene across geemap by replacing 9 bare except: clauses with except Exception: in several modules, reducing the chance of unintentionally catching BaseException subclasses (e.g., KeyboardInterrupt, SystemExit).
Changes:
- Replaced bare
except:withexcept Exception:in 6 files (9 sites total). - Preserved existing fallback behaviors while avoiding overly-broad
BaseExceptioncatches. - Standardized exception syntax in several utility and widget-related helpers.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
geemap/timelapse.py |
Updates PIL import guard to avoid bare except:. |
geemap/map_widgets.py |
Updates color normalization conversion guard to avoid bare except:. |
geemap/geemap.py |
Updates font_size parsing guard in add_labels() to avoid bare except:. |
geemap/coreutils.py |
Updates widget construction fallbacks to avoid bare except:. |
geemap/conversion.py |
Updates JS-to-Python conversion operator handling guard to avoid bare except:. |
geemap/common.py |
Updates several compatibility/stat-parsing fallbacks to avoid bare except:. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except Exception: | ||
| pass |
| except Exception: | ||
| raise ValueError(f"Unable to convert rgb value to hex: {color}") |
| except Exception: | ||
| raise ValueError("font_size must be something like '10pt'") |
| try: | ||
| toolbar_button = ipywidgets.ToggleButton(**widget_args) | ||
| except: | ||
| except Exception: | ||
| widget_args.pop("layout") | ||
| toolbar_button = ipywidgets.ToggleButton(**widget_args) | ||
| toolbar_button.layout.width = "28px" |
| try: | ||
| close_button = ipywidgets.ToggleButton(**close_button_args) | ||
| except: | ||
| except Exception: | ||
| close_button_args.pop("layout") | ||
| close_button = ipywidgets.ToggleButton(**close_button_args) | ||
| close_button.layout.width = "28px" |
| except Exception: | ||
| print("continue...") |
| percentile_2 = min([stats[s]["percentile_2"] for s in stats]) | ||
| percentile_98 = max([stats[s]["percentile_98"] for s in stats]) | ||
| except: | ||
| except Exception: |
| try: | ||
| cmap = plt.get_cmap(cmap, k) | ||
| except: | ||
| except Exception: |
| try: | ||
| cmap = plt.get_cmap(cmap_name, n_class) | ||
| except: | ||
| except Exception: |
Replaces 9 bare
except:withexcept Exception:in 6 files: timelapse.py, map_widgets.py, conversion.py, coreutils.py, common.py, geemap.py.