Skip to content

Unify semantic legend params.#727

Merged
cvanelteren merged 21 commits into
Ultraplot:mainfrom
gepcel:testsemanticlegend
Jun 3, 2026
Merged

Unify semantic legend params.#727
cvanelteren merged 21 commits into
Ultraplot:mainfrom
gepcel:testsemanticlegend

Conversation

@gepcel
Copy link
Copy Markdown
Collaborator

@gepcel gepcel commented May 18, 2026

Addresses #726

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

❌ Patch coverage is 97.44246% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
ultraplot/legend.py 93.42% 4 Missing and 6 partials ⚠️

📢 Thoughts on this report? Let us know!

@gepcel
Copy link
Copy Markdown
Collaborator Author

gepcel commented May 19, 2026

Try something.

pre-commit.ci autofix

@gepcel gepcel added the Wait Suspending current issue until further notice label May 19, 2026
@gepcel
Copy link
Copy Markdown
Collaborator Author

gepcel commented May 20, 2026

There are at least two existing issues with this PR:

  1. Arguments can be passed through full names, aliases, or within handle_kw, kwargs and other parameters, which occasionally causes argument value overrides.
  2. Regarding color-related parameters such as color, fc, ec and so on, I have not yet figured out a clear judgment logic. It should determine whether a value is a list of color values lthat should be applied separately to each entry, or whether list-type tuple values need to be converted into a single unified value. I suspect Ultraplot already has built-in logic to handle this judgment, but I have not located the relevant code implementation yet.

@gepcel gepcel added the help wanted Extra attention is needed label May 20, 2026
gepcel added 2 commits May 20, 2026 16:11
pass edgecolor ends up mulple args. Try unifying.
@gepcel gepcel force-pushed the testsemanticlegend branch from 28198b2 to c876eb0 Compare May 20, 2026 08:56
@gepcel
Copy link
Copy Markdown
Collaborator Author

gepcel commented May 21, 2026

call for

pre-commit.ci autofix

@gepcel gepcel removed the Wait Suspending current issue until further notice label May 24, 2026
Drop the debug `print()` in `_is_color_like` that fired twice for every
RGBA tuple, accept 3- or 4-float lists symmetrically with tuples so
`color=[0.5, 0.5, 0.5]` no longer crashes, and remove the redundant
`if check_color and _is_color_like(val)` branch in `_style_lookup` whose
two arms returned the same value.

Split the eight numbered steps inside `_pop_entry_props` into three small
helpers (`_pop_aliases`, `_pop_plurals`, `_pop_line2d_setters`) and reuse
them from `_pop_num_props`. Replace the cryptic `# without this, line 645
of test_legend.py won't pass` comment with an explanation of which
validator path actually depends on `label` / `labels` surviving, and the
`LegendEntry.__init__` MarkerStyle branch gets a one-paragraph rationale.

Misleading commented-out `ec` / `fc` lines come out of `_LINE_ALIAS_MAP`
(they already work through ultraplot's existing `_pop_props`), and the
short docstrings that the rewrite dropped on `UltraLegend.catlegend`,
`sizelegend`, `numlegend`, `geolegend` and `entrylegend` are restored.
Fix the typo introduced when the new semantic-legend test module was
added. Pure rename; no contents change.
PR Ultraplot#727 expands the semantic-legend surface (singular/plural aliases,
list/dict per-entry resolution, RGBA-tuple disambiguation, advanced
`MarkerStyle` properties on markers, patch-style passthrough on
`geolegend` / `numlegend`) but the public-facing `Axes.*` wrappers
still carry only one-line forwarder docstrings.

Register four snippets in `legend.py` — `legend.semantic_style_arg`,
`legend.semantic_style_kwargs`, `legend.semantic_num_style_kwargs`,
`legend.semantic_handle_kw` — capturing the resolution rules and style
vocabulary once. Wire the existing `docstring._snippet_manager` lazy
loader to import `ultraplot.legend` on demand, then apply full
numpy-style docstrings on `Axes.catlegend`, `entrylegend`,
`sizelegend`, `numlegend` and `geolegend` using those snippets, so the
same per-entry semantics and style alias table are documented in one
place across five methods.
@cvanelteren
Copy link
Copy Markdown
Collaborator

Thanks for the initiative @gepcel. Took a pass at tightening this up. First commit is the substantive cleanup of the legend rewrite — the stray print() in _is_color_like is gone, lists of three or four floats now work as a single color the same way tuples do (so color=[0.5, 0.5, 0.5] stops crashing), and a dead branch in _style_lookup that returned the same value either way is collapsed. The eight numbered steps inside _pop_entry_props are split into three small helpers that _pop_num_props reuses, the # without this, line 645 of test_legend.py won't pass comment is replaced with one that actually explains what it's guarding, and the misleading ec/fc lines come out of _LINE_ALIAS_MAP since they already work through _pop_props. The second commit is just renaming test_sematic_legend.py to fix the typo. The third commit adds real docs: a few shared snippets in legend.py feed numpy-style docstrings on the five public Axes.*legend methods, so the aliasvocabulary and per-entry rules live in one place instead of five.

I hope this helps you on their way, and I think this addresses most of the issues you outlined. Let me know!

@gepcel
Copy link
Copy Markdown
Collaborator Author

gepcel commented Jun 3, 2026

I agree.

@cvanelteren cvanelteren self-assigned this Jun 3, 2026
@cvanelteren cvanelteren removed the help wanted Extra attention is needed label Jun 3, 2026
@cvanelteren
Copy link
Copy Markdown
Collaborator

Leave it up to you if we can merger @gepcel

@gepcel
Copy link
Copy Markdown
Collaborator Author

gepcel commented Jun 3, 2026

Leave it up to you if we can merger @gepcel

I think it's ready to merge.

@cvanelteren cvanelteren merged commit 5e8247b into Ultraplot:main Jun 3, 2026
18 checks passed
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.

2 participants