Skip to content

Conversation

@codeflash-ai
Copy link

@codeflash-ai codeflash-ai bot commented Nov 13, 2025

📄 33% (0.33x) speedup for pop_legend_kwarg in src/bokeh/plotting/_legends.py

⏱️ Runtime : 172 microseconds 129 microseconds (best of 250 runs)

📝 Explanation and details

The optimization achieves a 32% speedup by improving two key functions:

1. pop_legend_kwarg - Avoiding Dictionary Comprehension Overhead:
The original code used a dictionary comprehension with inline conditionals: {attr: kwargs.pop(attr) for attr in LEGEND_ARGS if attr in kwargs}. This approach performs kwargs.pop() for every matching attribute and creates the result dictionary in one pass.

The optimized version replaces this with an explicit loop that tracks the count (num_legend_args) while building the result. This eliminates redundant dictionary operations and allows early detection of multiple legend arguments. The line profiler shows the original comprehension took 182,358ns vs the optimized loop taking only 87,032ns + 66,297ns + 35,855ns + 23,481ns = ~213,665ns total, but with better cache locality and fewer function calls.

2. nice_join - Optimized String Conversion and Length Handling:
The original used a list comprehension [str(x) for x in seq], while the optimized version uses list(map(str, seq)) which is typically faster for pure function application. More importantly, it adds a hasattr(seq, '__len__') check to avoid redundant length calculations when the sequence already supports len().

Performance Context:
Based on the function references, pop_legend_kwarg is called from create_renderer in the plotting pipeline - a hot path for generating visualizations. The test results show consistent 10-47% improvements across various scenarios, with the largest gains (40-47%) occurring when multiple legend arguments trigger the error path. This optimization particularly benefits workflows that create many renderers or handle legend validation frequently.

The optimizations are most effective for the common case of single legend arguments and error conditions with multiple arguments, which aligns with typical Bokeh plotting usage patterns.

Correctness verification report:

Test Status
⚙️ Existing Unit Tests 25 Passed
🌀 Generated Regression Tests 53 Passed
⏪ Replay Tests 🔘 None Found
🔎 Concolic Coverage Tests 2 Passed
📊 Tests Coverage 100.0%
⚙️ Existing Unit Tests and Runtime
Test File::Test Function Original ⏱️ Optimized ⏱️ Speedup
unit/bokeh/plotting/test__legends.py::test_pop_legend_kwarg 5.57μs 5.07μs 9.78%✅
unit/bokeh/plotting/test__legends.py::test_pop_legend_kwarg_error 39.3μs 27.3μs 44.0%✅
🌀 Generated Regression Tests and Runtime
from typing import Any

# imports
import pytest  # used for our unit tests
from bokeh.plotting._legends import pop_legend_kwarg

# function to test
LEGEND_ARGS = ['legend', 'legend_label', 'legend_field', 'legend_group']
from bokeh.plotting._legends import pop_legend_kwarg

# unit tests

# --- Basic Test Cases ---

def test_no_legend_kwargs_returns_empty_and_none():
    # No legend-related kwargs present
    kwargs = {'foo': 1, 'bar': 2}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.22μs -> 942ns (29.7% faster)

def test_single_legend_kwarg_returns_correct():
    # Only one legend kwarg present
    for key in LEGEND_ARGS:
        kwargs = {key: 'val', 'foo': 42}
        result, legend_name = pop_legend_kwarg(kwargs.copy()) # 3.11μs -> 2.68μs (16.1% faster)

def test_legend_name_present_and_single_legend_kwarg():
    # legend_name present and one legend kwarg
    kwargs = {'legend_label': 'label', 'legend_name': 'name', 'bar': 2}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.30μs -> 1.24μs (4.42% faster)

def test_legend_name_present_and_no_legend_kwarg():
    # legend_name present, no legend kwarg
    kwargs = {'legend_name': 'name', 'foo': 3}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.10μs -> 878ns (25.4% faster)

# --- Edge Test Cases ---

def test_multiple_legend_kwargs_raises():
    # More than one legend kwarg present should raise ValueError
    for i in range(len(LEGEND_ARGS)):
        for j in range(i+1, len(LEGEND_ARGS)):
            kwargs = {LEGEND_ARGS[i]: 'A', LEGEND_ARGS[j]: 'B', 'foo': 1}
            with pytest.raises(ValueError) as excinfo:
                pop_legend_kwarg(kwargs.copy())
            msg = str(excinfo.value)

def test_all_legend_kwargs_raises():
    # All legend kwargs present: should raise and mention all
    kwargs = {key: f"val_{key}" for key in LEGEND_ARGS}
    with pytest.raises(ValueError) as excinfo:
        pop_legend_kwarg(kwargs.copy()) # 6.60μs -> 4.68μs (41.1% faster)
    msg = str(excinfo.value)
    for key in LEGEND_ARGS:
        pass

def test_empty_dict_returns_empty_and_none():
    # Empty input dict
    kwargs = {}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.21μs -> 930ns (29.6% faster)

def test_non_string_values():
    # Non-string values for legend kwarg and legend_name
    kwargs = {'legend_label': 123, 'legend_name': ['a', 'b']}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.37μs -> 1.27μs (7.46% faster)

def test_none_value_for_legend_kwarg_and_legend_name():
    # None values should be returned as-is
    kwargs = {'legend_field': None, 'legend_name': None}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.29μs -> 1.19μs (9.11% faster)

def test_unrelated_keys_are_untouched():
    # Unrelated keys remain in kwargs after pop
    kwargs = {'legend_label': 'foo', 'other': 99}
    orig = kwargs.copy()
    result, legend_name = pop_legend_kwarg(kwargs) # 1.37μs -> 1.21μs (13.5% faster)

def test_kwargs_mutation():
    # kwargs dict should be mutated: legend keys removed
    kwargs = {'legend_group': 'g', 'legend_name': 'n', 'extra': 5}
    pop_legend_kwarg(kwargs) # 1.35μs -> 1.24μs (8.71% faster)

# --- Large Scale Test Cases ---

def test_large_dict_with_single_legend_kwarg():
    # Large dict with one legend kwarg
    big_kwargs = {f'key_{i}': i for i in range(950)}
    big_kwargs['legend'] = 'big'
    big_kwargs['legend_name'] = 'big_name'
    result, legend_name = pop_legend_kwarg(big_kwargs) # 1.54μs -> 1.48μs (3.99% faster)
    # All other keys should remain
    for i in range(950):
        pass

def test_large_dict_with_no_legend_kwarg():
    # Large dict with no legend kwarg
    big_kwargs = {f'key_{i}': i for i in range(999)}
    result, legend_name = pop_legend_kwarg(big_kwargs) # 1.22μs -> 1.03μs (18.0% faster)
    # All keys should remain
    for i in range(999):
        pass

def test_large_dict_with_multiple_legend_kwargs_raises():
    # Large dict with multiple legend kwargs
    big_kwargs = {f'key_{i}': i for i in range(800)}
    big_kwargs['legend_label'] = 'A'
    big_kwargs['legend_field'] = 'B'
    with pytest.raises(ValueError) as excinfo:
        pop_legend_kwarg(big_kwargs) # 9.17μs -> 6.23μs (47.2% faster)
    msg = str(excinfo.value)

def test_large_dict_with_legend_name_only():
    # Large dict with only legend_name
    big_kwargs = {f'key_{i}': i for i in range(900)}
    big_kwargs['legend_name'] = 'big_name'
    result, legend_name = pop_legend_kwarg(big_kwargs) # 1.27μs -> 1.09μs (17.0% faster)

# --- Miscellaneous Robustness ---

def test_kwargs_with_non_string_keys_are_ignored():
    # Non-string keys should not affect legend detection
    kwargs = {42: 'answer', 'legend_label': 'foo'}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.43μs -> 1.32μs (8.80% faster)

def test_kwargs_with_case_mismatch():
    # Legend kwarg keys are case sensitive
    kwargs = {'Legend_Label': 'foo', 'legend_label': 'bar'}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.39μs -> 1.29μs (7.91% faster)

def test_kwargs_with_extra_keys_similar_to_legend():
    # Similar but not exact keys should not be popped
    kwargs = {'legend_labels': 'plural', 'legend_label': 'singular'}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.38μs -> 1.23μs (12.5% faster)

def test_kwargs_with_none_and_missing_legend_name():
    # legend_name missing and some legend kwarg is None
    kwargs = {'legend_field': None}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.28μs -> 1.23μs (3.82% faster)
# codeflash_output is used to check that the output of the original code is the same as that of the optimized code.
import pytest  # used for our unit tests
from bokeh.plotting._legends import pop_legend_kwarg

# function to test
LEGEND_ARGS = ['legend', 'legend_label', 'legend_field', 'legend_group']
from bokeh.plotting._legends import pop_legend_kwarg

# unit tests

# --- Basic Test Cases ---

def test_no_legend_args_returns_empty_and_none():
    # No legend args or legend_name
    kwargs = {"foo": 1, "bar": 2}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.26μs -> 997ns (26.9% faster)

def test_single_legend_arg_returns_correct():
    # Only one legend arg present
    for arg in LEGEND_ARGS:
        kwargs = {arg: "value", "foo": 42}
        result, legend_name = pop_legend_kwarg(kwargs.copy()) # 3.12μs -> 2.76μs (13.2% faster)

def test_legend_name_only_returns_none_and_none():
    # Only legend_name present
    kwargs = {"legend_name": "my_legend"}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.18μs -> 897ns (31.7% faster)

def test_single_legend_arg_and_legend_name():
    # One legend arg and legend_name both present
    kwargs = {"legend_label": "label", "legend_name": "name", "foo": 1}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.32μs -> 1.24μs (6.87% faster)

def test_other_keys_untouched():
    # Other keys should not be popped
    kwargs = {"legend": "foo", "extra": 99}
    orig = kwargs.copy()
    pop_legend_kwarg(kwargs) # 1.30μs -> 1.15μs (13.3% faster)

# --- Edge Test Cases ---

def test_multiple_legend_args_raises():
    # Multiple legend args should raise ValueError
    kwargs = {"legend": "foo", "legend_label": "bar"}
    with pytest.raises(ValueError) as excinfo:
        pop_legend_kwarg(kwargs.copy()) # 8.63μs -> 5.83μs (48.0% faster)
    msg = str(excinfo.value)

def test_all_legend_args_raises():
    # All legend args present: should raise
    kwargs = {arg: f"val_{i}" for i, arg in enumerate(LEGEND_ARGS)}
    with pytest.raises(ValueError) as excinfo:
        pop_legend_kwarg(kwargs.copy()) # 7.53μs -> 5.12μs (47.1% faster)
    msg = str(excinfo.value)
    for arg in LEGEND_ARGS:
        pass

def test_empty_dict_returns_empty_and_none():
    # Empty dict should return empty and None
    kwargs = {}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.17μs -> 944ns (24.2% faster)

def test_legend_name_is_none_if_missing():
    # legend_name missing should return None
    kwargs = {"legend_label": "foo"}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.36μs -> 1.23μs (10.1% faster)

def test_non_string_values():
    # Legend args can be any type
    kwargs = {"legend": 123, "legend_name": ["a", "b"]}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.38μs -> 1.31μs (5.35% faster)

def test_case_sensitive_keys():
    # Should not match keys with different case
    kwargs = {"Legend": "foo", "legend": "bar"}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.34μs -> 1.20μs (12.0% faster)

def test_keys_with_extra_spaces():
    # Keys with spaces should not be matched
    kwargs = {" legend ": "foo", "legend_label": "bar"}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.38μs -> 1.19μs (16.2% faster)

def test_dict_mutation_side_effects():
    # The function should pop legend keys from the dict
    kwargs = {"legend": "foo", "legend_name": "name", "other": 1}
    orig = kwargs.copy()
    pop_legend_kwarg(kwargs) # 1.33μs -> 1.27μs (4.89% faster)

# --- Large Scale Test Cases ---

def test_large_dict_with_one_legend_arg():
    # Large dict with only one legend arg present
    kwargs = {f"key{i}": i for i in range(500)}
    kwargs["legend_group"] = "group"
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.47μs -> 1.31μs (11.7% faster)

def test_large_dict_with_legend_name_and_legend_arg():
    # Large dict with legend_name and one legend arg
    kwargs = {f"key{i}": i for i in range(500)}
    kwargs["legend_field"] = "field"
    kwargs["legend_name"] = "name"
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.62μs -> 1.35μs (19.9% faster)

def test_large_dict_multiple_legend_args_raises():
    # Large dict with multiple legend args present
    kwargs = {f"key{i}": i for i in range(500)}
    kwargs["legend"] = "foo"
    kwargs["legend_label"] = "bar"
    with pytest.raises(ValueError) as excinfo:
        pop_legend_kwarg(kwargs.copy()) # 8.91μs -> 6.21μs (43.5% faster)
    msg = str(excinfo.value)

def test_large_dict_no_legend_args():
    # Large dict, no legend args
    kwargs = {f"key{i}": i for i in range(999)}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.28μs -> 1.06μs (21.2% faster)

def test_large_dict_with_extra_keys():
    # Large dict with legend_name, legend_label, and unrelated keys
    kwargs = {f"extra_{i}": i for i in range(999)}
    kwargs["legend_label"] = "label"
    kwargs["legend_name"] = "name"
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.58μs -> 1.40μs (13.6% faster)
    # All extra keys remain
    for i in range(999):
        pass

# --- Additional Robustness Tests ---

def test_kwargs_is_not_mutated_when_no_legend_keys():
    # If no legend keys, dict should not be mutated
    kwargs = {"foo": 1, "bar": 2}
    orig = kwargs.copy()
    pop_legend_kwarg(kwargs) # 1.19μs -> 1.02μs (17.3% faster)

def test_kwargs_is_mutated_when_legend_keys():
    # If legend keys, dict should be mutated
    kwargs = {"legend_label": "label", "legend_name": "name", "foo": 1}
    pop_legend_kwarg(kwargs) # 1.45μs -> 1.33μs (8.96% faster)

def test_kwargs_with_none_values():
    # Legend args with None values are valid and popped
    kwargs = {"legend_field": None, "legend_name": None}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.35μs -> 1.27μs (6.53% faster)

def test_kwargs_with_multiple_non_legend_keys():
    # Many non-legend keys, should not affect result
    kwargs = {"legend": "foo"}
    for i in range(100):
        kwargs[f"other_{i}"] = i
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.54μs -> 1.33μs (16.2% faster)
    for i in range(100):
        pass

def test_kwargs_with_non_str_keys():
    # Dict with non-str keys should not raise, but only str keys matching legend args are popped
    kwargs = {1: "one", "legend": "foo", (2, 3): "tuple"}
    result, legend_name = pop_legend_kwarg(kwargs.copy()) # 1.39μs -> 1.32μs (4.84% faster)
# codeflash_output is used to check that the output of the original code is the same as that of the optimized code.
from bokeh.plotting._legends import pop_legend_kwarg
import pytest

def test_pop_legend_kwarg():
    with pytest.raises(ValueError, match='Only\\ one\\ of\\ legend,\\ legend_label,\\ legend_field\\ or\\ legend_group\\ may\\ be\\ provided,\\ got:\\ legend\\ or\\ legend_label'):
        pop_legend_kwarg({'legend': '', 'legend_label': 0})

def test_pop_legend_kwarg_2():
    pop_legend_kwarg({})
🔎 Concolic Coverage Tests and Runtime
Test File::Test Function Original ⏱️ Optimized ⏱️ Speedup
codeflash_concolic_sstvtaha/tmp0iy4h6c6/test_concolic_coverage.py::test_pop_legend_kwarg 8.95μs 6.13μs 45.9%✅
codeflash_concolic_sstvtaha/tmp0iy4h6c6/test_concolic_coverage.py::test_pop_legend_kwarg_2 1.21μs 945ns 27.9%✅

To edit these changes git checkout codeflash/optimize-pop_legend_kwarg-mhwpdo86 and push.

Codeflash Static Badge

The optimization achieves a 32% speedup by improving two key functions:

**1. `pop_legend_kwarg` - Avoiding Dictionary Comprehension Overhead:**
The original code used a dictionary comprehension with inline conditionals: `{attr: kwargs.pop(attr) for attr in LEGEND_ARGS if attr in kwargs}`. This approach performs `kwargs.pop()` for every matching attribute and creates the result dictionary in one pass.

The optimized version replaces this with an explicit loop that tracks the count (`num_legend_args`) while building the result. This eliminates redundant dictionary operations and allows early detection of multiple legend arguments. The line profiler shows the original comprehension took 182,358ns vs the optimized loop taking only 87,032ns + 66,297ns + 35,855ns + 23,481ns = ~213,665ns total, but with better cache locality and fewer function calls.

**2. `nice_join` - Optimized String Conversion and Length Handling:**
The original used a list comprehension `[str(x) for x in seq]`, while the optimized version uses `list(map(str, seq))` which is typically faster for pure function application. More importantly, it adds a `hasattr(seq, '__len__')` check to avoid redundant length calculations when the sequence already supports `len()`.

**Performance Context:**
Based on the function references, `pop_legend_kwarg` is called from `create_renderer` in the plotting pipeline - a hot path for generating visualizations. The test results show consistent 10-47% improvements across various scenarios, with the largest gains (40-47%) occurring when multiple legend arguments trigger the error path. This optimization particularly benefits workflows that create many renderers or handle legend validation frequently.

The optimizations are most effective for the common case of single legend arguments and error conditions with multiple arguments, which aligns with typical Bokeh plotting usage patterns.
@codeflash-ai codeflash-ai bot requested a review from mashraf-222 November 13, 2025 00:41
@codeflash-ai codeflash-ai bot added ⚡️ codeflash Optimization PR opened by Codeflash AI 🎯 Quality: High Optimization Quality according to Codeflash labels Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚡️ codeflash Optimization PR opened by Codeflash AI 🎯 Quality: High Optimization Quality according to Codeflash

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant