ronantakizawa/codereview-bench huggingface dataset:
Repository: plotly/dash
File: dash/dash_table/__init__.py (Python)

Code before:
from __future__ import print_function

import os as _os
import sys as _sys
import json

import dash as _dash

if not hasattr(_dash, "__plotly_dash") and not hasattr(_dash, "development"):
    print(
        "Dash was not successfully imported. "
        "Make sure you don't have a file "
        'named \n"dash.py" in your current directory.',
        file=_sys.stderr,
    )
    _sys.exit(1)

from ._imports_ import *  # noqa: E402, F401, F403
from ._imports_ import __all__  # noqa: E402

_basepath = _os.path.dirname(__file__)
_filepath = _os.path.abspath(_os.path.join(_basepath, "package-info.json"))
with open(_filepath) as f:
    package = json.load(f)

package_name = package["name"].replace(" ", "_").replace("-", "_")

Review comment (suggestion):
Ah, there will be a little merge conflict with https://github.com/plotly/dash/pull/1734 where we remove all these build artifacts... (`dash/dash_table`, `dash/dcc`, `dash/html`)

Code after:
import os as _os
import sys as _sys
import json

import dash as _dash

if not hasattr(_dash, "__plotly_dash") and not hasattr(_dash, "development"):
    print(
        "Dash was not successfully imported. "
        "Make sure you don't have a file "
        'named \n"dash.py" in your current directory.',
        file=_sys.stderr,
    )
    _sys.exit(1)

from ._imports_ import *  # noqa: E402, F401, F403
from ._imports_ import __all__  # noqa: E402

_basepath = _os.path.dirname(__file__)
_filepath = _os.path.abspath(_os.path.join(_basepath, "package-info.json"))
with open(_filepath) as f:
    package = json.load(f)

package_name = package["name"].replace(" ", "_").replace("-", "_")
__version__ = package["version"]
Repository: plotly/dash
File: components/dash-core-components/src/utils/computeSliderMarkers.js (JavaScript)

Code before:
            marks.length >= 2 &&
            max - marks[marks.length - 2] <= interval * discardThreshold
        ) {
            marks.pop();
        }
    }
    const marksObject = {};
    marks.forEach(mark => {
        marksObject[mark] = applyD3Format(mark, min, max);
    });
    marksObject[min] = applyD3Format(min, min, max);
    marksObject[max] = applyD3Format(max, min, max);
    return marksObject;
};

/**
 * - Auto generate marks if not given,
 * - Not generate anything at all when explicit null is given to marks
 * - Then truncate marks so no out of range marks
 */
export const sanitizeMarks = ({min, max, marks, step}) => {
    if (marks === null) {
        return undefined;
    }

    const [min_mark, max_mark] = Object.values(setUndefined(min, max, marks));

    const truncated_marks =
        marks && isEmpty(marks) === false
            ? truncateMarks(min_mark, max_mark, marks)
            : marks;

    if (truncated_marks && isEmpty(truncated_marks) === false) {
        return truncated_marks;
    }

    return autoGenerateMarks(min_mark, max_mark, step);
};

/**
 * Calculate default value if not defined
 */
export const calcValue = (min, max, value) => {
    if (value !== undefined) {
        return value;
    }

    return [min, max];
};


Review comment (suggestion):
```suggestion
    const {min_mark, max_mark} = setUndefined(min, max, marks);
```
Does the same thing right? Except the previous way is dependent on `Object.values` always putting `min_mark` first, which is implementation-dependent (implementation of JS itself and our implementation of `setUndefined`)

Code after:
            marks.length >= 2 &&
            max - marks[marks.length - 2] <= interval * discardThreshold
        ) {
            marks.pop();
        }
    }
    const marksObject = {};
    marks.forEach(mark => {
        marksObject[mark] = applyD3Format(mark, min, max);
    });
    marksObject[min] = applyD3Format(min, min, max);
    marksObject[max] = applyD3Format(max, min, max);
    return marksObject;
};

/**
 * - Auto generate marks if not given,
 * - Not generate anything at all when explicit null is given to marks
 * - Then truncate marks so no out of range marks
 */
export const sanitizeMarks = ({min, max, marks, step}) => {
    if (marks === null) {
        return undefined;
    }

    const {min_mark, max_mark} = setUndefined(min, max, marks);

    const truncated_marks =
        marks && isEmpty(marks) === false
            ? truncateMarks(min_mark, max_mark, marks)
            : marks;

    if (truncated_marks && isEmpty(truncated_marks) === false) {
        return truncated_marks;
    }

    return autoGenerateMarks(min_mark, max_mark, step);
};

/**
 * Calculate default value if not defined
 */
export const calcValue = (min, max, value) => {
    if (value !== undefined) {
        return value;
    }

    return [min, max];
};
Repository: plotly/dash
File: components/dash-core-components/src/fragments/Graph.react.js (JavaScript)

Code before:
                setProps({restyleData: restyle});
            }
        });
        gd.on('plotly_unhover', () => {
            if (clear_on_unhover) {
                setProps({hoverData: null});
            }
        });
    }

    getStyle() {
        const {responsive} = this.props;
        let {style} = this.props;

        // When there is no forced responsive style, return the original style property
        if (!responsive) {
            return style;
        }

        // Otherwise, if the height is not set, we make the graph size equal to the parent one
        if (!style) {
            style = {};
        }

        if (!style.height) {
            style.height = '100%';
        }

        return style;
    }

    componentDidMount() {
        this.plot(this.props);
        if (this.props.prependData) {
            this.mergeTraces(this.props, 'prependData', 'prependTraces');
        }
        if (this.props.extendData) {
            this.mergeTraces(this.props, 'extendData', 'extendTraces');
        }

        if (this.props.prependData?.length || this.props.extendData?.length) {
            this.props._dashprivate_onFigureModified(this.props.figure);
        }
    }

    componentWillUnmount() {
        const gd = this.gd.current;
        if (gd && gd.removeAllListeners) {
            gd.removeAllListeners();
            if (this._hasPlotted) {
                Plotly.purge(gd);

Review comment (suggestion):
```suggestion
            return Object.assign({height: '100%'}, style);
```

We shouldn't be modifying `style` in place, could cause issues if the user turns off `responsive`, or reads `style` back out in another callback.


Code after:

    isResponsive(props) {
        const {config, figure, responsive} = props;

        if (type(responsive) === 'Boolean') {
            return responsive;
        }

        return Boolean(
            config.responsive &&
                (!figure.layout ||
                    ((figure.layout.autosize ||
                        isNil(figure.layout.autosize)) &&
                        (isNil(figure.layout.height) ||
                            isNil(figure.layout.width))))
        );
    }

    graphResize(force = false) {
        if (!force && !this.isResponsive(this.props)) {
            return;
        }

        const gd = this.gd.current;
        if (!gd) {
            return;
        }

        gd.classList.add('dash-graph--pending');

        Plotly.Plots.resize(gd)
            .catch(() => {})
            .finally(() => gd.classList.remove('dash-graph--pending'));
    }

    bindEvents() {
        const {
            setProps,
            clear_on_unhover,
            relayoutData,
            restyleData,
            hoverData,
            selectedData,
        } = this.props;

        const gd = this.gd.current;

        gd.on('plotly_click', eventData => {
            const clickData = filterEventData(gd, eventData, 'click');
            if (!isNil(clickData)) {
                setProps({clickData});
Repository: plotly/dash
File: components/dash-table/tests/selenium/test_sizing_x.py (Python)

Code before:
import pytest

from test_sizing import on_focus

from utils import (
    basic_modes,
    generate_mock_data,
)


@pytest.mark.parametrize("props", basic_modes)
@pytest.mark.parametrize(
    "data_fn",
    [generate_mock_data],
)
def test_szng004_on_focus(test, props, data_fn):
    on_focus(test, props, data_fn)


Review comment (suggestion):
The `data_fn` parameter can be removed as there is now only one.

Code after:
import pytest

from test_sizing import on_focus

from utils import (
    basic_modes,
    generate_mock_data,
)


@pytest.mark.parametrize("props", basic_modes)
def test_szng004_on_focus(test, props):
    on_focus(test, props, generate_mock_data)
Repository: plotly/dash
File: components/dash-core-components/src/fragments/Dropdown.react.js (JavaScript)

Code before:
                if (!values.includes(selectedValue)) {
                    setProps({value: null});
                }
            }
            setOptionsCheck(sanitizedOptions);
        }
    }, [sanitizedOptions, optionsCheck, multi, value, selectedValue]);

    return (
        <div
            id={id}
            className="dash-dropdown"
            style={style}
            data-dash-is-loading={
                (loading_state && loading_state.is_loading) || undefined
            }
        >
            <ReactDropdown
                filterOptions={filterOptions}
                options={sanitizeOptions(options)}
                value={selectedValue}
                onChange={onChange}
                onInputChange={onInputChange}
                backspaceRemoves={clearable}
                deleteRemoves={clearable}
                inputProps={{autoComplete: 'of'}}
                {...omit(['setProps', 'value', 'options'], props)}
            />
        </div>
    );
};

Dropdown.propTypes = propTypes;
Dropdown.defaultProps = defaultProps;

export default Dropdown;


Review comment (suggestion):
```suggestion
                inputProps={{autoComplete: 'off'}}
```

Code after:
                const invalids = value.filter(v => !values.includes(v));
                if (invalids.length) {
                    setProps({value: without(invalids, value)});
                }
            } else {
                if (!values.includes(value)) {
                    setProps({value: null});
                }
            }
            setOptionsCheck(sanitizedOptions);
        }
    }, [sanitizedOptions, optionsCheck, multi, value]);

    return (
        <div
            id={id}
            className="dash-dropdown"
            style={style}
            data-dash-is-loading={
                (loading_state && loading_state.is_loading) || undefined
            }
        >
            <ReactDropdown
                filterOptions={filterOptions}
                options={sanitizeOptions(options)}
                value={value}
                onChange={onChange}
                onInputChange={onInputChange}
                backspaceRemoves={clearable}
                deleteRemoves={clearable}
                inputProps={{autoComplete: 'off'}}
                {...pick(RDProps, props)}
            />
        </div>
    );
};

Dropdown.propTypes = propTypes;
Dropdown.defaultProps = defaultProps;

export default Dropdown;
Repository: plotly/dash
File: components/dash-core-components/src/fragments/Dropdown.react.js (JavaScript)

Code before:
                    setProps({value: null});
                }
            }
            setOptionsCheck(sanitizedOptions);
        }
    }, [sanitizedOptions, optionsCheck, multi, value]);

    return (
        <div
            id={id}
            className="dash-dropdown"
            style={style}
            data-dash-is-loading={
                (loading_state && loading_state.is_loading) || undefined
            }
        >
            <ReactDropdown
                filterOptions={filterOptions}
                options={sanitizeOptions(options)}
                value={value}
                onChange={onChange}
                onInputChange={onInputChange}
                backspaceRemoves={clearable}
                deleteRemoves={clearable}
                inputProps={{autoComplete: 'off'}}
                {...omit(['setProps', 'value', 'options'], props)}
            />
        </div>
    );
};

Dropdown.propTypes = propTypes;
Dropdown.defaultProps = defaultProps;

export default Dropdown;


Review comment (refactor):
Thanks for converting this component to functional - and of course getting rid of one `UNSAFE_` method 🎉  That will become very important as soon as someone tries to use Dash with React 18 (which is officially out as of a few weeks ago)

FWIW not necessary to include in this PR, but just because I see it carried over in your refactoring, I'd like us to eventually get rid of all the `ramda.omit` calls fowarding props to underlying components https://github.com/plotly/dash-core-components/issues/703

Code after:
    );

    useEffect(() => {
        if (optionsCheck !== sanitizedOptions && !isNil(value)) {
            const values = sanitizedOptions.map(option => option.value);
            if (multi && Array.isArray(value)) {
                const invalids = value.filter(v => !values.includes(v));
                if (invalids.length) {
                    setProps({value: without(invalids, value)});
                }
            } else {
                if (!values.includes(value)) {
                    setProps({value: null});
                }
            }
            setOptionsCheck(sanitizedOptions);
        }
    }, [sanitizedOptions, optionsCheck, multi, value]);

    return (
        <div
            id={id}
            className="dash-dropdown"
            style={style}
            data-dash-is-loading={
                (loading_state && loading_state.is_loading) || undefined
            }
        >
            <ReactDropdown
                filterOptions={filterOptions}
                options={sanitizeOptions(options)}
                value={value}
                onChange={onChange}
                onInputChange={onInputChange}
                backspaceRemoves={clearable}
                deleteRemoves={clearable}
                inputProps={{autoComplete: 'off'}}
                {...pick(RDProps, props)}
            />
        </div>
    );
};

Dropdown.propTypes = propTypes;
Dropdown.defaultProps = defaultProps;

export default Dropdown;
Repository: plotly/dash
File: dash/dash-renderer/src/actions/callbacks.ts (TypeScript)

Code before:

        if (typeof returnValue?.then === 'function') {
            returnValue = await returnValue;
        }

        zipIfArray(outputs, returnValue).forEach(([outi, reti]) => {
            zipIfArray(outi, reti).forEach(([outij, retij]) => {
                const {id, property} = outij;
                const idStr = stringifyId(id);
                const dataForId = (result[idStr] = result[idStr] || {});
                if (retij !== dc.no_update) {
                    dataForId[property] = retij;
                }
            });
        });
    } catch (e) {
        if (e === dc.PreventUpdate) {
            status = STATUS.PREVENT_UPDATE;
        } else {
            status = STATUS.CLIENTSIDE_ERROR;
            throw e;
        }
    } finally {
        // Setting server = client forces network = 0
        const totalTime = Date.now() - requestTime;
        const resources = {
            __dash_server: totalTime,
            __dash_client: totalTime,
            __dash_upload: 0,
            __dash_download: 0
        };

        if (config.ui) {
            dispatch(
                updateResourceUsage({
                    id: payload.output,
                    usage: resources,
                    status,
                    result,
                    inputs,
                    state
                })
            );
        }
    }

    return result;
}

function handleServerside(
    dispatch: any,

Review comment (suggestion):
Oh actually, we might want to keep the delete here too - so that if there's an error in a sync callback we still delete the context.

Code after:

        if (typeof returnValue?.then === 'function') {
            returnValue = await returnValue;
        }

        zipIfArray(outputs, returnValue).forEach(([outi, reti]) => {
            zipIfArray(outi, reti).forEach(([outij, retij]) => {
                const {id, property} = outij;
                const idStr = stringifyId(id);
                const dataForId = (result[idStr] = result[idStr] || {});
                if (retij !== dc.no_update) {
                    dataForId[property] = retij;
                }
            });
        });
    } catch (e) {
        if (e === dc.PreventUpdate) {
            status = STATUS.PREVENT_UPDATE;
        } else {
            status = STATUS.CLIENTSIDE_ERROR;
            throw e;
        }
    } finally {
        delete dc.callback_context;

        // Setting server = client forces network = 0
        const totalTime = Date.now() - requestTime;
        const resources = {
            __dash_server: totalTime,
            __dash_client: totalTime,
            __dash_upload: 0,
            __dash_download: 0
        };

        if (config.ui) {
            dispatch(
                updateResourceUsage({
                    id: payload.output,
                    usage: resources,
                    status,
                    result,
                    inputs,
                    state
                })
            );
        }
    }

    return result;
}
Repository: plotly/dash
File: tests/integration/devtools/test_devtools_ui.py (Python)

Code before:
        [html.P(id="just_an_id", children="You should never see this")]
    )

    # create alternative response, for the endpoint '/'
    # servering an alternative response, will disable further `before_request` functions e.g. those by dash
    @app.server.before_request
    def create_an_alternative_response():
        if flask.request.endpoint == "/":
            return flask.Response(
                '<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">\n'
                "<title>Alternative repsonse</title>\n"
                '<h1 id="alternative_id">Alternative response header</h1>\n',
                200,
                mimetype="text/html",
            )

    dash_thread_server.start(
        app,
        debug=True,
        use_reloader=False,
        use_debugger=True,
        dev_tools_hot_reload=False,
    )

    driver.get(dash_thread_server.url)
    dash_br.find_element("alternative_id", attribute="ID")


Review comment (suggestion):
```suggestion
    dash_br.find_element("#alternative_id")
```
I don't mind giving folks the option to use `ID` and others, but within this repo, when a CSS selector is easy I'd rather we standardize.

Code after:
        [html.P(id="just_an_id", children="You should never see this")]
    )

    # create alternative response, for the endpoint '/'
    # servering an alternative response, will disable further `before_request` functions e.g. those by dash
    @app.server.before_request
    def create_an_alternative_response():
        if flask.request.endpoint == "/":
            return flask.Response(
                '<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">\n'
                "<title>Alternative repsonse</title>\n"
                '<h1 id="alternative_id">Alternative response header</h1>\n',
                200,
                mimetype="text/html",
            )

    dash_thread_server.start(
        app,
        debug=True,
        use_reloader=False,
        use_debugger=True,
        dev_tools_hot_reload=False,
    )

    driver.get(dash_thread_server.url)
    dash_br.find_element("#alternative_id")
Repository: plotly/dash
File: components/dash-core-components/tests/integration/dropdown/test_visibility.py (Python)

Code before:
                columns=[
                    {"name": x, "id": x, "selectable": True} for x in ["a", "b", "c"]
                ],
                editable=True,
                row_deletable=True,
                fixed_rows=dict(headers=True),
                fixed_columns=dict(headers=True),
                data=[
                    {"a": "a" + str(x), "b": "b" + str(x), "c": "c" + str(x)}
                    for x in range(0, 20)
                ],
            ),
        ]
    )

    dash_duo.start_server(app)

    dash_duo.find_element("#dropdown").click()
    dash_duo.wait_for_element("#dropdown .Select-menu-outer")

    dash_duo.percy_snapshot("dcc.Dropdown dropdown overlaps table fixed rows/columns")

    assert dash_duo.get_logs() == []


@pytest.mark.DCC788
def test_ddvi002_maxHeight(dash_duo):
    app = Dash(__name__)
    app.layout = Div(
        [Dropdown([str(i) for i in range(100)], "1", id="dropdown", maxHeight=800)]
    )

    dash_duo.start_server(app)

    dash_duo.find_element("#dropdown").click()
    dash_duo.wait_for_element("#dropdown .Select-menu-outer")

    dash_duo.percy_snapshot("dcc.Dropdown dropdown menu maxHeight 800px")

    assert dash_duo.get_logs() == []


Review comment (bug):
```suggestion
```
Those marks were intended to match tests with specific bugs they locked down - personally though I find that cryptic and not that useful, I'd rather we just add a comment if it's not clear what the test is testing.

Code after:
                columns=[
                    {"name": x, "id": x, "selectable": True} for x in ["a", "b", "c"]
                ],
                editable=True,
                row_deletable=True,
                fixed_rows=dict(headers=True),
                fixed_columns=dict(headers=True),
                data=[
                    {"a": "a" + str(x), "b": "b" + str(x), "c": "c" + str(x)}
                    for x in range(0, 20)
                ],
            ),
        ]
    )

    dash_duo.start_server(app)

    dash_duo.find_element("#dropdown").click()
    dash_duo.wait_for_element("#dropdown .Select-menu-outer")

    dash_duo.percy_snapshot("dcc.Dropdown dropdown overlaps table fixed rows/columns")

    assert dash_duo.get_logs() == []


def test_ddvi002_maxHeight(dash_duo):
    app = Dash(__name__)
    app.layout = Div(
        [Dropdown([str(i) for i in range(100)], "1", id="dropdown", maxHeight=800)]
    )

    dash_duo.start_server(app)

    dash_duo.find_element("#dropdown").click()
    dash_duo.wait_for_element("#dropdown .Select-menu-outer")

    dash_duo.percy_snapshot("dcc.Dropdown dropdown menu maxHeight 800px")

    assert dash_duo.get_logs() == []