Fix three stale documented defaults (TopoJson.overlay, TimeSliderChoropleth.overlay, GeoJsonPopup.localize) - #2276
Open
dylanpulver wants to merge 1 commit into
Conversation
TopoJson.overlay, TimeSliderChoropleth.overlay and GeoJsonPopup.localize each override a default and kept the parameter description they were copied from, so the docstring states the old value: TopoJson.overlay documented False, defaults True TimeSliderChoropleth.overlay documented False, defaults True GeoJsonPopup.localize documented False, defaults True The code is right in each case. Layer documents and defaults overlay=False and GeoJson correctly documents its True override; GeoJsonTooltip correctly documents localize=False. Only these three drifted. The new test walks every class in the package and compares the documented boolean default against the signature default for overlay, control, show and localize.
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.
Three classes override a default and kept the parameter description they were copied from, so the documentation states the old value:
TopoJson.overlayFalseTrueTimeSliderChoropleth.overlayFalseTrueGeoJsonPopup.localizeFalseTrueIn each case the code is right and the docstring is stale, and the tie is broken by a sibling in the same file rather than by preference:
Layer(map.py) both documents and defaultsoverlay=False.GeoJsonoverrides it toTrueand documentsTrue.TopoJsonandTimeSliderChoroplethoverride it toTrueand keptLayer's line.GeoJsonTooltipdocumentslocalizeasFalseand defaults toFalse.GeoJsonPopupoverrides toTrueand kept the tooltip's line.overlaydecides whether a layer shows up inLayerControlas a toggleable overlay or as a base layer, so this is a wrong mental model rather than a cosmetic slip.The new test walks every class in the package — 96 of them — and compares the documented boolean default with the signature default for
overlay,control,showandlocalize. It is restricted to those four because a bareTrue/Falsein a docstring is unambiguous to compare, whereas the numeric and string defaults elsewhere are written in too many styles to check mechanically without false positives.Verification: with the three docstrings reverted to their current text, exactly those three parametrised cases fail and the other 94 pass, so the check is neither vacuous nor over-broad. There is also a small self-check asserting the docstring parser really does detect a mismatch.
black26.5.1 andruff0.16.3 (the versions pinned in.pre-commit-config.yaml) report the three files unchanged and clean.Suite run locally, Python 3.14, excluding
tests/selenium: 369 passed, 2 failed —test_icon_invalid_marker_colorsandtest_valid_png_size, both of which fail identically on unmodifiedmainhere, so they look like environment rather than regressions.tests/snapshotscould not be collected at all locally becausepixelmatchis not installed; that path is untested by me.