Skip to content

Fix three stale documented defaults (TopoJson.overlay, TimeSliderChoropleth.overlay, GeoJsonPopup.localize) - #2276

Open
dylanpulver wants to merge 1 commit into
python-visualization:mainfrom
dylanpulver:fix/stale-documented-defaults
Open

Fix three stale documented defaults (TopoJson.overlay, TimeSliderChoropleth.overlay, GeoJsonPopup.localize)#2276
dylanpulver wants to merge 1 commit into
python-visualization:mainfrom
dylanpulver:fix/stale-documented-defaults

Conversation

@dylanpulver

Copy link
Copy Markdown

Three classes override a default and kept the parameter description they were copied from, so the documentation states the old value:

documented actually defaults to
TopoJson.overlay False True
TimeSliderChoropleth.overlay False True
GeoJsonPopup.localize False True

In 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 defaults overlay=False. GeoJson overrides it to True and documents True. TopoJson and TimeSliderChoropleth override it to True and kept Layer's line.
  • GeoJsonTooltip documents localize as False and defaults to False. GeoJsonPopup overrides to True and kept the tooltip's line.

overlay decides whether a layer shows up in LayerControl as 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, show and localize. It is restricted to those four because a bare True/False in 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. black 26.5.1 and ruff 0.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_colors and test_valid_png_size, both of which fail identically on unmodified main here, so they look like environment rather than regressions. tests/snapshots could not be collected at all locally because pixelmatch is not installed; that path is untested by me.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant