lib: fix incorrect backend detection in get_backend()#7240
lib: fix incorrect backend detection in get_backend()#7240Photon079 wants to merge 2 commits intoOSGeo:mainfrom
Conversation
Previously, get_backend() would return 'ipyleaflet' whenever folium import failed, without checking if the object was an ipyleaflet.Map. This could cause crashes in Layer.add_to() methods when wrong backend was selected. This fix: - Properly checks object types for both folium and ipyleaflet - Raises clear ValueError when neither backend matches - Adds comprehensive tests covering all edge cases - Follows GRASS coding standards Bug discovered while writing tests - original logic made dangerous assumptions that could break map functionality. AI assistance used for codebase understanding and test design. All code changes and testing were performed by the contributor.
Photon079
changed the title
github-actions
Bot
added
Python
Photon079
changed the title
petrasovaa
left a comment
petrasovaa
left a comment
There was a problem hiding this comment.
Please use pre-commit to fix any formatting.
| import folium | ||
| import ipyleaflet |
There was a problem hiding this comment.
The tests will hard-fail in environments without folium/ipyleaflet instead of being skipped gracefully
| error_msg = "No supported mapping backend found" | ||
| raise ValueError(error_msg) |
There was a problem hiding this comment.
Just make it a single line.
Also, the message is somewhat misleading, it sounds like a missing library, not a wrong object type.
There was a problem hiding this comment.
Correction, this may be enforced by a ruff rule, so this is probably correct.
|
hey @petrasovaa thank you for reviewing , i am working on these things and will update soon |
* Skip tests gracefully when mapping libraries are missing * Update ValueError exception message wording * Fix docstring alignment in get_backend()
Photon079
force-pushed
the
fix/get-backend-bug
branch
from
2608440 to
df5731d
Compare
|
Hey @petrasovaa, thanks for the review! I've pushed updates to address your feedback: Used pytest.importorskip so tests skip gracefully if the mapping libraries are missing. |
2 participants
Previously,
get_backend()returned"ipyleaflet"whenfoliumimport failed, without validating whether the object was actually anipyleaflet.Map. This could lead to incorrect backend selection and failures in layer operations.This fix:
This issue was discovered while writing tests for
grass.jupyteras part of the GSoC test-of-skills tasks. I have also opened issue #7239 for tracking this bug.