Repository navigation
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.
petrasovaa
requested changes
Apr 7, 2026
petrasovaa
left a comment
Contributor
There was a problem hiding this comment.
Please use pre-commit to fix any formatting.
Author
|
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
April 8, 2026 19:27
2608440 to
df5731d
Compare
Author
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.