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

Code before:
    make_grouping_by_index,
    grouping_len,
)
from ._utils import (
    create_callback_id,
    stringify_id,
    to_json,
    coerce_to_list,
    AttributeDict,
)

from . import _validate
from .long_callback.managers import BaseLongCallbackManager
from ._callback_context import context_value


class NoUpdate:
    def to_plotly_json(self):  # pylint: disable=no-self-use
        return {"_dash_no_update": "_dash_no_update"}

    @staticmethod
    def is_no_update(obj):
        return (
            isinstance(obj, NoUpdate)
            or isinstance(obj, dict)
            and obj == {"_dash_no_update": "_dash_no_update"}
        )


GLOBAL_CALLBACK_LIST = []
GLOBAL_CALLBACK_MAP = {}
GLOBAL_INLINE_SCRIPTS = []


# pylint: disable=too-many-locals
def callback(
    *_args,
    background=False,
    interval=1000,
    progress=None,
    progress_default=None,
    running=None,
    cancel=None,
    manager=None,
    cache_args_to_ignore=None,
    **_kwargs,
):
    """
    Normally used as a decorator, `@dash.callback` provides a server-side
    callback relating the values of one or more `Output` items to one or
    more `Input` items which will trigger the callback when they change,

Review comment (suggestion):
```suggestion
            or (isinstance(obj, dict)
            and obj == {"_dash_no_update": "_dash_no_update"})
```
that's not going to pass linting... but the point is it's hard to read if I have to try and remember (or learn) that `and` has higher precedence than `or`

Code after:
    make_grouping_by_index,
    grouping_len,
)
from ._utils import (
    create_callback_id,
    stringify_id,
    to_json,
    coerce_to_list,
    AttributeDict,
)

from . import _validate
from .long_callback.managers import BaseLongCallbackManager
from ._callback_context import context_value


class NoUpdate:
    def to_plotly_json(self):  # pylint: disable=no-self-use
        return {"_dash_no_update": "_dash_no_update"}

    @staticmethod
    def is_no_update(obj):
        return isinstance(obj, NoUpdate) or (
            isinstance(obj, dict) and obj == {"_dash_no_update": "_dash_no_update"}
        )


GLOBAL_CALLBACK_LIST = []
GLOBAL_CALLBACK_MAP = {}
GLOBAL_INLINE_SCRIPTS = []


# pylint: disable=too-many-locals
def callback(
    *_args,
    background=False,
    interval=1000,
    progress=None,
    progress_default=None,
    running=None,
    cancel=None,
    manager=None,
    cache_args_to_ignore=None,
    **_kwargs,
):
    """
    Normally used as a decorator, `@dash.callback` provides a server-side
    callback relating the values of one or more `Output` items to one or
    more `Input` items which will trigger the callback when they change,
    and optionally `State` items which provide additional information but
    do not trigger the callback directly.
Repository: plotly/dash
File: components/dash-table/src/dash-table/dash/DataTable.js (JavaScript)

Code before:
import * as R from 'ramda';
import React, { Component, Suspense } from 'react';
import PropTypes from 'prop-types';
import { asyncDecorator } from '@plotly/dash-component-plugins';

import LazyLoader from 'dash-table/LazyLoader';

/**
 * Dash DataTable is an interactive table component designed for
 * viewing, editing, and exploring large datasets.
 * DataTable is rendered with standard, semantic HTML <table/> markup,
 * which makes it accessible, responsive, and easy to style. This
 * component was written from scratch in React.js specifically for the
 * Dash community. Its API was designed to be ergonomic and its behavior
 * is completely customizable through its properties.
 */
export default class DataTable extends Component {
    render() {
        return (
            <Suspense fallback={null}>
                <RealDataTable {...this.props} />
            </Suspense>
        );
    }
}

const RealDataTable = asyncDecorator(DataTable, LazyLoader.table);

export const defaultProps = {

Review comment (bug):
The lint job failed, I think our settings actually prohibit these spaces. `npm run format` from `components/dash-table` should sort this out.

Code after:
import * as R from 'ramda';
import React, {Component, Suspense} from 'react';
import PropTypes from 'prop-types';
import {asyncDecorator} from '@plotly/dash-component-plugins';

import LazyLoader from 'dash-table/LazyLoader';

/**
 * Dash DataTable is an interactive table component designed for
 * viewing, editing, and exploring large datasets.
 * DataTable is rendered with standard, semantic HTML <table/> markup,
 * which makes it accessible, responsive, and easy to style. This
 * component was written from scratch in React.js specifically for the
 * Dash community. Its API was designed to be ergonomic and its behavior
 * is completely customizable through its properties.
 */
export default class DataTable extends Component {
    render() {
        return (
            <Suspense fallback={null}>
                <RealDataTable {...this.props} />
            </Suspense>
        );
    }
}

const RealDataTable = asyncDecorator(DataTable, LazyLoader.table);

export const defaultProps = {
Repository: plotly/dash
File: tests/integration/renderer/test_component_as_prop.py (Python)

Code before:
        ]
    )

    @app.callback(
        Output("options", "options"),
        Input("add-option", "n_clicks"),
        State("options", "options"),
        prevent_initial_call=True,
    )
    def add_option(_, options):
        return [*options, opt(str(uuid.uuid4()))]

    @app.callback(
        Output({"type": "text", "index": MATCH}, "children"),
        Input({"type": "button", "index": MATCH}, "n_clicks"),
    )
    def demo(n_clicks):
        return n_clicks

    dash_duo.start_server(app)

    dash_duo.wait_for_element("#add-option").click()
    for i in range(1, n + 2):
        dash_duo.wait_for_element(f"#options label:nth-child({i}) button").click()
        dash_duo.wait_for_text_to_equal(f"#options label:nth-child({i}) span", "1")


Review comment (suggestion):
```suggestion
        dash_duo.wait_for_text_to_equal(f"#options label:nth-child({i}) span", "")
        dash_duo.wait_for_element(f"#options label:nth-child({i}) button").click()
        dash_duo.wait_for_text_to_equal(f"#options label:nth-child({i}) span", "1")
```
Just a minor addition: make sure the nth button click was really the reason the nth span changed from empty to "1"

Code after:
        ]
    )

    @app.callback(
        Output("options", "options"),
        Input("add-option", "n_clicks"),
        State("options", "options"),
        prevent_initial_call=True,
    )
    def add_option(_, options):
        return [*options, opt(str(uuid.uuid4()))]

    @app.callback(
        Output({"type": "text", "index": MATCH}, "children"),
        Input({"type": "button", "index": MATCH}, "n_clicks"),
    )
    def demo(n_clicks):
        return n_clicks

    dash_duo.start_server(app)

    dash_duo.wait_for_element("#add-option").click()
    for i in range(1, n + 2):
        dash_duo.wait_for_text_to_equal(f"#options label:nth-child({i}) span", "")
        dash_duo.wait_for_element(f"#options label:nth-child({i}) button").click()
        dash_duo.wait_for_text_to_equal(f"#options label:nth-child({i}) span", "1")
Repository: plotly/dash
File: components/dash-core-components/src/components/Geolocation.react.js (JavaScript)

Code before:
import {Component} from 'react';
import PropTypes from 'prop-types';

/**
 * The CurrentLocation component gets geolocation of the device from the web browser.  See more info here:
 * https://developer.mozilla.org/en-US/docs/Web/API/Geolocation_API
 */

export default class Geolocation extends Component {
    constructor(props) {
        super(props);
        this.success = this.success.bind(this);
        this.error = this.error.bind(this);
        this.updatePosition = this.updatePosition.bind(this);
    }
    updatePosition() {
        if (this.props.update_now) {
            this.props.setProps({
                update_now: false,
            });
        }
        if (!navigator.geolocation) {
            this.error({
                code: 999,
                message: 'Your browser does not support Geolocation',
            });
            if (this.props.show_alert) {
                alert(`ERROR(${this.error.code}): ${this.error.message}`);
            }
        } else {
            const positionOptions = {
                enableHighAccuracy: this.props.high_accuracy,
                maximumAge: this.props.maximum_age,
                timeout: this.props.timeout,
            };

            navigator.geolocation.getCurrentPosition(
                this.success,
                this.error,

Review comment (suggestion):
`updatePosition` doesn't require a bind, it's not used as pointer.

Code after:
import {Component} from 'react';
import PropTypes from 'prop-types';

/**
 * The CurrentLocation component gets geolocation of the device from the web browser.  See more info here:
 * https://developer.mozilla.org/en-US/docs/Web/API/Geolocation_API
 */

export default class Geolocation extends Component {
    constructor(props) {
        super(props);
        this.success = this.success.bind(this);
        this.error = this.error.bind(this);
    }
    updatePosition() {
        if (this.props.update_now) {
            this.props.setProps({
                update_now: false,
            });
        }
        if (!navigator.geolocation) {
            this.error({
                code: 999,
                message: 'Your browser does not support Geolocation',
            });
        } else {
            const positionOptions = {
                enableHighAccuracy: this.props.high_accuracy,
                maximumAge: this.props.maximum_age,
                timeout: this.props.timeout,
            };

            navigator.geolocation.getCurrentPosition(
                this.success,
                this.error,
                positionOptions
            );
        }
    }
Repository: plotly/dash
File: tests/integration/multi_page/test_pages_order.py (Python)

Code before:
                        )
                    )
                    for page in dash.page_registry.values()
                ]
            ),
            dash.page_container,
            dcc.Location(id="url", refresh=True),
        ]
    )

    modules = [
        "multi_layout3",
        "multi_layout2",
        "multi_layout1",
        "multi_layout4",
        "multi_layout3.5",
        "multi_layout5",
        "multi_layout6",
        "multi_layout7",
        "multi_layout8",
        "multi_layout9",
        "multi_layout10",
        "multi_layout11",
        "multi_layout12",
        "multi_layout13",
        "multi_layout16",
        "multi_layout14",
        "multi_layout15",
        "pages.defaults",
        "pages.metas",
        "pages.not_found_404",
        "pages.page1",
        "pages.page2",
        "pages.path_variables",
        "pages.query_string",
        "pages.redirect",
    ]

    dash_duo.start_server(app)

    assert (
        list(dash.page_registry) == modules
    ), "check order of modules in dash.page_registry"

    # There should be no `validation_layout` when suppress_callback_exceptions=True`
    assert app.validation_layout is None

    assert dash_duo.get_logs() == [], "browser console should contain no error"


Review comment (suggestion):
```suggestion
        "first_string_order",
        "yyy_last_string_order",
        "aaa_first_with_no_order",
```

Code after:
    )

    app.layout = html.Div(
        [
            html.Div(
                [
                    html.Div(
                        dcc.Link(
                            f"{page['name']} - {page['path']}",
                            id=page["id"],
                            href=page["path"],
                        )
                    )
                    for page in dash.page_registry.values()
                ]
            ),
            dash.page_container,
            dcc.Location(id="url", refresh=True),
        ]
    )

    modules = [
        "multi_layout3",
        "multi_layout2",
        "multi_layout1",
        "multi_layout4",
        "multi_layout3.5",
        "multi_layout5",
        "multi_layout6",
        "multi_layout7",
        "multi_layout8",
        "multi_layout9",
        "multi_layout10",
        "multi_layout11",
        "multi_layout12",
        "multi_layout13",
        "first_string_order",
        "yyy_last_string_order",
        "aaa_first_with_no_order",
        "multi_layout14",
        "multi_layout15",
        "pages.defaults",
        "pages.metas",
        "pages.not_found_404",
        "pages.page1",
        "pages.page2",
        "pages.path_variables",
        "pages.query_string",
        "pages.redirect",
    ]
Repository: plotly/dash
File: components/dash-core-components/tests/integration/input/test_input_basics.py (Python)

Code before:
    assert dash_dcc.get_logs() == []


def test_inbs003_styles_are_scoped(dash_dcc):
    app = Dash(__name__)

    app.index_string = """
    <html>
        <body>
            <input id="ExternalInput" required />
            {%app_entry%}
            {%config%}
            {%scripts%}
            {%renderer%}
        </body>
    </html>
    """

    app.layout = html.Div(
        className="test-input-css",
        children=[dcc.Input(id="DashInput", required=True, className="unittest")],
    )

    dash_dcc.start_server(app)

    external_input = dash_dcc.driver.find_element(By.ID, "ExternalInput")
    dash_input = dash_dcc.driver.find_element(By.CLASS_NAME, "unittest")

    external_outline_css = external_input.value_of_css_property("outline")
    dash_outline_css = dash_input.value_of_css_property("outline")

    assert external_outline_css != dash_outline_css


Review comment (suggestion):
You can use `dash_dcc.find_element("#ExternalInput")` instead of the driver and By object.

Code after:


def test_inbs003_styles_are_scoped(dash_dcc):
    app = Dash(__name__)

    app.index_string = """
    <html>
        <body>
            <input id="ExternalInput" required />
            {%app_entry%}
            {%config%}
            {%scripts%}
            {%renderer%}
        </body>
    </html>
    """

    app.layout = html.Div(
        className="test-input-css",
        children=[dcc.Input(id="DashInput", required=True, className="unittest")],
    )

    dash_dcc.start_server(app)

    external_input = dash_dcc.find_element("#ExternalInput")
    dash_input = dash_dcc.find_element(".unittest")

    external_outline_css = external_input.value_of_css_property("outline")
    dash_outline_css = dash_input.value_of_css_property("outline")

    assert external_outline_css != dash_outline_css
Repository: plotly/dash
File: tests/integration/renderer/test_request_hooks.py (Python)

Code before:
    _in.send_keys("fired request again")

    dash_duo.wait_for_text_to_equal("#output-1", "fired request again")
    dash_duo.wait_for_text_to_equal("#output-token", "..")

    assert len(dash_duo.get_logs()) == 2


def test_rdrh004_layout_hooks(dash_duo):
    app = Dash(__name__)

    app.index_string = """<!DOCTYPE html>
    <html>
        <head>
            {%metas%}
            <title>{%title%}</title>
            {%favicon%}
            {%css%}
        </head>
        <body>
            {%app_entry%}
            <footer>
                {%config%}
                {%scripts%}
                <script id="_dash-renderer" type"application/json">
                    const renderer = new DashRenderer({
                        layout_pre: () => {
                            var layoutPre = document.createElement('div');
                            layoutPre.setAttribute('id', 'layout-pre');
                            layoutPre.innerHTML = 'layout_pre generated this text';
                            document.body.appendChild(layoutPre);
                        },
                        layout_post: (response) => {
                            response.props.children = "layout_post generated this text";
                        }
                    })
                </script>
            </footer>
        </body>
    </html>"""

    app.layout = html.Div(id="layout")

    dash_duo.start_server(app)

    dash_duo.wait_for_text_to_equal("#layout-pre", "layout_pre generated this text")
    dash_duo.wait_for_text_to_equal("#layout", "layout_post generated this text")

    assert dash_duo.get_logs() == []


Review comment (suggestion):
This api usage is not something I think we should expand upon without improvements, it's really hard to use and has boilerplate not required for the feature.

I suggest we add parameter(s) that get them in the main index without the need to override the index string.

Code after:

    dash_duo.wait_for_text_to_equal("#output-1", "fired request again")
    dash_duo.wait_for_text_to_equal("#output-token", "..")

    assert len(dash_duo.get_logs()) == 2


def test_rdrh004_layout_hooks(dash_duo):
    hooks: RendererHooks = {
        "layout_pre": """
            () => {
                var layoutPre = document.createElement('div');
                layoutPre.setAttribute('id', 'layout-pre');
                layoutPre.innerHTML = 'layout_pre generated this text';
                document.body.appendChild(layoutPre);
            }
        """,
        "layout_post": """
            (response) => {
                response.props.children = "layout_post generated this text";
            }
        """,
    }

    app = Dash(__name__, hooks=hooks)
    app.layout = html.Div(id="layout")

    dash_duo.start_server(app)

    dash_duo.wait_for_text_to_equal("#layout-pre", "layout_pre generated this text")
    dash_duo.wait_for_text_to_equal("#layout", "layout_post generated this text")

    assert dash_duo.get_logs() == []
Repository: plotly/dash
File: tests/integration/callbacks/test_dynamic_callback.py (Python)

Code before:
    )

    @app.callback(
        Output("output", "children"),
        Input("create", "n_clicks"),
        _allow_dynamic_callbacks=True,
        prevent_initial_call=True,
    )
    def on_click(n_clicks):
        @app.callback(
            Output("output-2", "children"),
            Input("dynamic", "n_clicks"),
            prevent_initial_call=True,
        )
        def on_click2(n_clicks2):
            return f"Dynamic clicks {n_clicks2}"

        return f"creator {n_clicks}"

    dash_duo.start_server(app)

    dash_duo.wait_for_element("#create").click()
    dash_duo.wait_for_text_to_equal("#output", "creator 1")

    dash_duo.wait_for_element("#dynamic").click()
    dash_duo.wait_for_text_to_equal("#output-2", "Dynamic clicks 1")


Review comment (suggestion):
```suggestion
    dash_duo.wait_for_element("#dynamic").click()
    dash_duo.wait_for_element("#create").click()
    dash_duo.wait_for_text_to_equal("#output", "creator 1")
    dash_duo.wait_for_text_to_equal("#output-2", "initial")

    dash_duo.wait_for_element("#dynamic").click()
    dash_duo.wait_for_text_to_equal("#output-2", "Dynamic clicks 2")
```
Just to make it crystal clear that the dynamic callback was not attached at first.

Code after:
            html.Div("initial", id="output-2"),
            html.Button("dynamic", id="dynamic"),
        ]
    )

    @app.callback(
        Output("output", "children"),
        Input("create", "n_clicks"),
        _allow_dynamic_callbacks=True,
        prevent_initial_call=True,
    )
    def on_click(n_clicks):
        @app.callback(
            Output("output-2", "children"),
            Input("dynamic", "n_clicks"),
            prevent_initial_call=True,
        )
        def on_click2(n_clicks2):
            return f"Dynamic clicks {n_clicks2}"

        return f"creator {n_clicks}"

    dash_duo.start_server(app)

    dash_duo.wait_for_element("#dynamic").click()
    dash_duo.wait_for_element("#create").click()
    dash_duo.wait_for_text_to_equal("#output", "creator 1")
    dash_duo.wait_for_text_to_equal("#output-2", "initial")

    dash_duo.wait_for_element("#dynamic").click()
    dash_duo.wait_for_text_to_equal("#output-2", "Dynamic clicks 2")
Repository: plotly/dash
File: dash/dash-renderer/src/utils/clientsideFunctions.ts (TypeScript)

Code before:
import {updateProps, notifyObservers} from '../actions/index';
import {getPath} from '../actions/paths';

const set_props = (updates: []) => {
    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
    // @ts-ignore
    const ds = (window.dash_stores = window.dash_stores || []);
    for (let y = 0; y < ds.length; y++) {
        const {dispatch, getState} = ds[y];
        const {paths} = getState();
        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
        // @ts-ignore
        updates.forEach(({id, ...props}) => {
            const componentPath = getPath(paths, id);
            dispatch(
                updateProps({
                    props,
                    itempath: componentPath
                })
            );
            dispatch(notifyObservers({id, props}));
        });
    }
};

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const dc = (window.dash_clientside = window.dash_clientside || {});
dc['set_props'] = set_props;


Review comment (performance):
I think the api for this could be simpler without the batching of updates and extracting the id from the props into a function argument.
The function signature would then look like:
```
set_props(id: string | object, props: {[k: string]: any}): void;
```



Code after:
import {updateProps, notifyObservers} from '../actions/index';
import {getPath} from '../actions/paths';

const set_props = (id: string | object, props: {[k: string]: any}) => {
    const ds = ((window as any).dash_stores =
        (window as any).dash_stores || []);
    for (let y = 0; y < ds.length; y++) {
        const {dispatch, getState} = ds[y];
        const {paths} = getState();
        const componentPath = getPath(paths, id);
        dispatch(
            updateProps({
                props,
                itempath: componentPath
            })
        );
        dispatch(notifyObservers({id, props}));
    }
};

const dc = ((window as any).dash_clientside =
    (window as any).dash_clientside || {});
dc['set_props'] = set_props;