ronantakizawa/codereview-bench huggingface dataset:
Repository: pyscript/pyscript
File: pyscript.core/src/stdlib/pyscript/magic_js.py (Python)

Code before:
import js as globalThis

from pyscript.util import NotSupported

RUNNING_IN_WORKER = not hasattr(globalThis, "document")

if RUNNING_IN_WORKER:
    import polyscript

    PyWorker = NotSupported(
        "pyscript.PyWorker",
        "pyscript.PyWorker works only when running in the main thread",
    )
    window = polyscript.xworker.window
    document = window.document
    sync = polyscript.xworker.sync

    # in workers the display does not have a default ID
    # but there is a sync utility from xworker
    def current_target():
        return polyscript.target

else:
    import _pyscript

    window = globalThis
    document = globalThis.document
    sync = NotSupported(
        "pyscript.sync", "pyscript.sync works only when running in a worker"
    )

    # in MAIN the current element target exist, just use it
    def current_target():
        return _pyscript.target


Review comment (refactor):
these kind of changes make we wish to remove any linter from our source code ... this effectively side-effects on the module as `PyWorker` won't exist anymore so if it's pre-commit doing this, please remove this file from its greedy and breaking changes, thank you.

Code after:
import js as globalThis
from pyscript.util import NotSupported

RUNNING_IN_WORKER = not hasattr(globalThis, "document")

if RUNNING_IN_WORKER:
    import polyscript

    PyWorker = NotSupported(
        "pyscript.PyWorker",
        "pyscript.PyWorker works only when running in the main thread",
    )
    window = polyscript.xworker.window
    document = window.document
    sync = polyscript.xworker.sync

    # in workers the display does not have a default ID
    # but there is a sync utility from xworker
    def current_target():
        return polyscript.target

else:
    import _pyscript
    from _pyscript import PyWorker

    window = globalThis
    document = globalThis.document
    sync = NotSupported(
        "pyscript.sync", "pyscript.sync works only when running in a worker"
    )

    # in MAIN the current element target exist, just use it
    def current_target():
        return _pyscript.target
Repository: pyscript/pyscript
File: pyscript.core/src/plugins/deprecations-manager.js (JavaScript)

Code before:
// PyScript Derepcations Plugin
import { hooks } from "../core.js";
import { notify } from "./error.js";

// eslint-disable-next-line no-unused-vars
hooks.main.onReady.add(function checkDeprecations(pyScript) {
    const scripts = document.querySelectorAll("script");
    for (const script of scripts) checkLoadingScriptsFromLatest(script.src);
});
// eslint-disable-next-line no-unused-vars

/**
 * Check if src being loaded from pyscript.net/latest and display a notification if true
 * * @param {string} src
 */
function checkLoadingScriptsFromLatest(src) {
    if (/\/pyscript\.net\/latest/.test(src)) {
        notify(
            "Loading scripts from latest is deprecated and will be removed soon. Please use a specific version instead.",
        );
    }
}


Review comment (refactor):
there are a few issues with this approach:

  * a worker won't trigger this hook ever ... this is only for py-script on the main thread. A solution is to add the same hook, still on main, but as `onWorker` so that whenever a worker script bootstraps the main can notify, if needed
  * scripts added at runtime or lazily due "*reasons*" could happen after another py-script landed ... and being that a different project/source of code I am not sure we need a Mutation Observer
  * we could notify ASAP instead of too late ... so I would actually move this function in the top level, add it to both `hooks.main.onReady` and `hooks.main.onWroker` (note both on `main`) but also execute it ASAP instead of waiting, as scripts with that source are likely already in the page.

thoughts?

Code after:
// PyScript Derepcations Plugin
import { hooks } from "../core.js";
import { notify } from "./error.js";

// react lazily on PyScript bootstrap
hooks.main.onReady.add(checkDeprecations);
hooks.main.onWorker.add(checkDeprecations);

/**
 * Check that there are no scripts loading from pyscript.net/latest
 */
function checkDeprecations() {
    const scripts = document.querySelectorAll("script");
    for (const script of scripts) checkLoadingScriptsFromLatest(script.src);
}

/**
 * Check if src being loaded from pyscript.net/latest and display a notification if true
 * * @param {string} src
 */
function checkLoadingScriptsFromLatest(src) {
    if (/\/pyscript\.net\/latest/.test(src)) {
        notify(
            "Loading scripts from latest is deprecated and will be removed soon. Please use a specific version instead.",
        );
    }
}
Repository: pyscript/pyscript
File: pyscript.core/src/stdlib/pyscript/magic_js.py (Python)

Code before:
import js as globalThis
from polyscript import js_modules
from pyscript.util import NotSupported

RUNNING_IN_WORKER = not hasattr(globalThis, "document")

if RUNNING_IN_WORKER:
    import js
    import polyscript

    PyWorker = NotSupported(
        "pyscript.PyWorker",
        "pyscript.PyWorker works only when running in the main thread",
    )
    window = polyscript.xworker.window
    document = window.document
    js.document = document
    sync = polyscript.xworker.sync

    # in workers the display does not have a default ID
    # but there is a sync utility from xworker
    def current_target():
        return polyscript.target

else:
    import _pyscript
    from _pyscript import PyWorker

    window = globalThis
    document = globalThis.document
    sync = NotSupported(
        "pyscript.sync", "pyscript.sync works only when running in a worker"
    )

    # in MAIN the current element target exist, just use it
    def current_target():

Review comment (suggestion):
```suggestion
class JSModule:
```
Doing `class A(object)` is a holdover from Python 2.x.

Code after:
import sys

import js as globalThis
from polyscript import js_modules
from pyscript.util import NotSupported

RUNNING_IN_WORKER = not hasattr(globalThis, "document")


# allow `from pyscript.js_modules.xxx import yyy`
class JSModule(object):
    def __init__(self, name):
        self.name = name

    def __getattr__(self, field):
        # avoid pyodide looking for non existent fields
        if not field.startswith("_"):
            return getattr(getattr(js_modules, self.name), field)


# generate N modules in the system that will proxy the real value
for name in globalThis.Reflect.ownKeys(js_modules):
    sys.modules[f"pyscript.js_modules.{name}"] = JSModule(name)
sys.modules["pyscript.js_modules"] = js_modules

if RUNNING_IN_WORKER:
    import js
    import polyscript

    PyWorker = NotSupported(
        "pyscript.PyWorker",
        "pyscript.PyWorker works only when running in the main thread",
    )
    window = polyscript.xworker.window
    document = window.document
    js.document = document
Repository: pyscript/pyscript
File: pyscript.core/src/stdlib/pyweb/pydom.py (Python)

Code before:
try:
    import warnings
except ImportError:
    # TODO: For now it probably means we are in MicroPython. We should figure
    # out the "right" way to handle this. For now we just ignore the warning
    # and logging to console
    class warnings:
        @staticmethod
        def warn(*args, **kwargs):
            print("WARNING: ", *args, **kwargs)

    warnings = None

try:
    from functools import cached_property
except ImportError:
    # TODO: same comment about micropython as above
    cached_property = property

try:
    from pyodide.ffi import JsProxy
except ImportError:
    # TODO: same comment about micropython as above
    def JsProxy(obj):
        return obj


from pyscript import display, document, window

alert = window.alert


class BaseElement:
    def __init__(self, js_element):
        self._js = js_element
        self._parent = None
        self.style = StyleProxy(self)
        self._proxies = {}

    def __eq__(self, obj):
        """Check if the element is the same as the other element by comparing
        the underlying JS element"""
        return isinstance(obj, BaseElement) and obj._js == self._js

    @property
    def parent(self):
        if self._parent:
            return self._parent

        if self._js.parentElement:
            self._parent = self.__class__(self._js.parentElement)

Review comment (suggestion):
my only concern here is *instance* brand check where it would always be `False` but maybe that's OK/desired ... if that's the case, this is fine, as long as `isinstance(obj, JsProxy)` won't break the world if executed.

Code after:
except ImportError:
    Any = "Any"

try:
    import warnings
except ImportError:
    # TODO: For now it probably means we are in MicroPython. We should figure
    # out the "right" way to handle this. For now we just ignore the warning
    # and logging to console
    class warnings:
        @staticmethod
        def warn(*args, **kwargs):
            print("WARNING: ", *args, **kwargs)


try:
    from functools import cached_property
except ImportError:
    # TODO: same comment about micropython as above
    cached_property = property

try:
    from pyodide.ffi import JsProxy
except ImportError:
    # TODO: same comment about micropython as above
    def JsProxy(obj):
        return obj


from pyscript import display, document, window

alert = window.alert


class BaseElement:
    def __init__(self, js_element):
        self._js = js_element
        self._parent = None
        self.style = StyleProxy(self)
        self._proxies = {}

    def __eq__(self, obj):
        """Check if the element is the same as the other element by comparing
        the underlying JS element"""
        return isinstance(obj, BaseElement) and obj._js == self._js

    @property
    def parent(self):
        if self._parent:
            return self._parent
Repository: pyscript/pyscript
File: pyscript.core/src/stdlib/pyscript/websocket.py (Python)

Code before:
                return value.to_py()
            # shims in MicroPython
            return memoryview(as_bytearray(value))

        return value


class WebSocket(object):
    CONNECTING = 0
    OPEN = 1
    CLOSING = 2
    CLOSED = 3

    def __init__(self, **kw):
        url = kw["url"]
        if protocols in kw:
            socket = js.WebSocket.new(url, kw[protocols])
        else:
            socket = js.WebSocket.new(url)
        object.__setattr__(self, "_ws", socket)

        set_attr = js.Reflect.set
        for t in ["onclose", "onerror", "onmessage", "onopen"]:
            if t in kw:
                # Pyodide fails at setting socket[t] directly
                set_attr(socket, t, create_proxy(kw[t]))

    def __getattr__(self, attr):
        return getattr(self._ws, attr)

    def __setattr__(self, attr, value):
        if attr == "onmessage":
            self._ws[attr] = lambda e: value(EventMessage(e))
        else:
            self._ws[attr] = value

    def close(self, **kw):
        if code in kw and reason in kw:
            self._ws.close(kw[code], kw[reason])
        elif code in kw:
            self._ws.close(kw[code])
        else:
            self._ws.close()

    def send(self, data):
        if isinstance(data, str):
            self._ws.send(data)
        else:
            buffer = js.Uint8Array.new(len(data))
            for pos, b in enumerate(data):
                buffer[pos] = b

Review comment (suggestion):
Python has a builtin called `setattr` that does this:
```suggestion
                setattr(socket, t, create_proxy(kw[t]))
```

Code after:
                return value.to_py()
            # shims in MicroPython
            return memoryview(as_bytearray(value))

        return value


class WebSocket(object):
    CONNECTING = 0
    OPEN = 1
    CLOSING = 2
    CLOSED = 3

    def __init__(self, **kw):
        url = kw["url"]
        if protocols in kw:
            socket = js.WebSocket.new(url, kw[protocols])
        else:
            socket = js.WebSocket.new(url)
        object.__setattr__(self, "_ws", socket)

        for t in ["onclose", "onerror", "onmessage", "onopen"]:
            if t in kw:
                # Pyodide fails at setting socket[t] directly
                setattr(socket, t, create_proxy(kw[t]))

    def __getattr__(self, attr):
        return getattr(self._ws, attr)

    def __setattr__(self, attr, value):
        if attr == "onmessage":
            self._ws[attr] = lambda e: value(EventMessage(e))
        else:
            self._ws[attr] = value

    def close(self, **kw):
        if code in kw and reason in kw:
            self._ws.close(kw[code], kw[reason])
        elif code in kw:
            self._ws.close(kw[code])
        else:
            self._ws.close()

    def send(self, data):
        if isinstance(data, str):
            self._ws.send(data)
        else:
            buffer = js.Uint8Array.new(len(data))
            for pos, b in enumerate(data):
                buffer[pos] = b
            self._ws.send(buffer)
Repository: pyscript/pyscript
File: pyscript.core/src/stdlib/pyscript/magic_js.py (Python)

Code before:
import json
import sys

import js as globalThis
from polyscript import config as _config
from polyscript import js_modules
from pyscript.util import NotSupported

RUNNING_IN_WORKER = not hasattr(globalThis, "document")

config = json.loads(globalThis.JSON.stringify(_config))


# allow `from pyscript.js_modules.xxx import yyy`
class JSModule:
    def __init__(self, name):
        self.name = name

    def __getattr__(self, field):
        # avoid pyodide looking for non existent fields
        if not field.startswith("_"):
            return getattr(getattr(js_modules, self.name), field)


# generate N modules in the system that will proxy the real value
for name in globalThis.Reflect.ownKeys(js_modules):
    sys.modules[f"pyscript.js_modules.{name}"] = JSModule(name)
sys.modules["pyscript.js_modules"] = js_modules

if RUNNING_IN_WORKER:
    import polyscript

    PyWorker = NotSupported(
        "pyscript.PyWorker",
        "pyscript.PyWorker works only when running in the main thread",
    )

    try:

Review comment (suggestion):
I usually normalise to: `if "micropython" in sys.version.lower()` But I don't think it makes much difference.

Code after:
import json
import sys

import js as globalThis
from polyscript import config as _config
from polyscript import js_modules
from pyscript.util import NotSupported

RUNNING_IN_WORKER = not hasattr(globalThis, "document")

config = json.loads(globalThis.JSON.stringify(_config))

if "MicroPython" in sys.version:
    config["type"] = "mpy"
else:
    config["type"] = "py"


# allow `from pyscript.js_modules.xxx import yyy`
class JSModule:
    def __init__(self, name):
        self.name = name

    def __getattr__(self, field):
        # avoid pyodide looking for non existent fields
        if not field.startswith("_"):
            return getattr(getattr(js_modules, self.name), field)


# generate N modules in the system that will proxy the real value
for name in globalThis.Reflect.ownKeys(js_modules):
    sys.modules[f"pyscript.js_modules.{name}"] = JSModule(name)
sys.modules["pyscript.js_modules"] = js_modules

if RUNNING_IN_WORKER:
    import polyscript

    PyWorker = NotSupported(
Repository: pytest-dev/pytest
File: testing/test_faulthandler.py (Python)

Code before:
    # ensure warning is emitted if faulthandler_timeout is configured
    warning_line = "*faulthandler.py*faulthandler module enabled before*"
    if faulthandler_timeout > 0:
        result.stdout.fnmatch_lines(warning_line)
    else:
        result.stdout.no_fnmatch_line(warning_line)
    result.stdout.fnmatch_lines("*1 passed*")
    assert result.ret == 0


def test_get_stderr_fileno_invalid_fd(monkeypatch) -> None:
    """Test for faulthandler being able to handle invalid file descriptors for stderr (#8249)."""
    from _pytest.faulthandler import FaultHandlerHooks

    class StdErrWrapper(io.StringIO):
        """
        Mimic ``twisted.logger.LoggingFile`` to simulate returning an invalid file descriptor.

        https://github.com/twisted/twisted/blob/twisted-20.3.0/src/twisted/logger/_io.py#L132-L139
        """

        def fileno(self):
            return -1

    wrapper = StdErrWrapper()
    monkeypatch.setattr("sys.stderr", wrapper)

    # Even when the stderr wrapper signals an invalid file descriptor,
    # ``_get_stderr_fileno()`` should return the real one.
    assert FaultHandlerHooks._get_stderr_fileno() == 2


Review comment (suggestion):
Since this test itself runs under pytest, it's probably safer to scope the stderr changes, like so:


```py
with pytest.MonkeyPatch.context() as mp:
   mp.setattr("sys.stderr", wrapper)
    # test here...
```

This way by the time the test is finished the stderr is restored.

Code after:
    # ensure warning is emitted if faulthandler_timeout is configured
    warning_line = "*faulthandler.py*faulthandler module enabled before*"
    if faulthandler_timeout > 0:
        result.stdout.fnmatch_lines(warning_line)
    else:
        result.stdout.no_fnmatch_line(warning_line)
    result.stdout.fnmatch_lines("*1 passed*")
    assert result.ret == 0


def test_get_stderr_fileno_invalid_fd() -> None:
    """Test for faulthandler being able to handle invalid file descriptors for stderr (#8249)."""
    from _pytest.faulthandler import FaultHandlerHooks

    class StdErrWrapper(io.StringIO):
        """
        Mimic ``twisted.logger.LoggingFile`` to simulate returning an invalid file descriptor.

        https://github.com/twisted/twisted/blob/twisted-20.3.0/src/twisted/logger/_io.py#L132-L139
        """

        def fileno(self):
            return -1

    wrapper = StdErrWrapper()

    with pytest.MonkeyPatch.context() as mp:
        mp.setattr("sys.stderr", wrapper)

        # Even when the stderr wrapper signals an invalid file descriptor,
        # ``_get_stderr_fileno()`` should return the real one.
        assert FaultHandlerHooks._get_stderr_fileno() == 2
Repository: pytest-dev/pytest
File: doc/en/conf.py (Python)

Code before:
# relative to this directory. They are copied after the builtin static files,
# so a file named "default.css" will overwrite the builtin "default.css".
# html_static_path = ['_static']

# If not '', a 'Last updated on:' timestamp is inserted at every page bottom,
# using the given strftime format.
# html_last_updated_fmt = '%b %d, %Y'

# If true, SmartyPants will be used to convert quotes and dashes to
# typographically correct entities.
# html_use_smartypants = True

# Custom sidebar templates, maps document names to template names.
# html_sidebars = {}
# html_sidebars = {'index': 'indexsidebar.html'}

html_sidebars = {
    "index": [
        "slim_searchbox.html",
        "sidebarintro.html",
        "globaltoc.html",
        "links.html",
        "sourcelink.html",
    ],
    "**": [
        "slim_searchbox.html",
        "globaltoc.html",
        "relations.html",
        "links.html",
        "sourcelink.html",
    ],
}

# Additional templates that should be rendered to pages, maps page names to
# template names.
# html_additional_pages = {}
# html_additional_pages = {'index': 'index.html'}


# If false, no module index is generated.
html_domain_indices = True

# If false, no index is generated.
html_use_index = False

# If true, the index is split into individual pages for each letter.
# html_split_index = False

# If true, links to the reST sources are added to the pages.
html_show_sourcelink = False


Review comment (suggestion):
Oh wow, this is a bad way to do things. See https://pradyunsg.me/furo/customisation/injecting/#css-or-js for the guidance from the theme for how to inject CSS.

Code after:
# html_use_smartypants = True

# Custom sidebar templates, maps document names to template names.
# html_sidebars = {}
# html_sidebars = {'index': 'indexsidebar.html'}

html_sidebars = {
    "index": [
        "sidebar/brand.html",
        "sidebar/search.html",
        "sidebar/scroll-start.html",
        "sidebarintro.html",
        "globaltoc.html",
        "links.html",
        "sidebar/scroll-end.html",
        "style.html",
    ],
    "**": [
        "sidebar/brand.html",
        "sidebar/search.html",
        "sidebar/scroll-start.html",
        "globaltoc.html",
        "relations.html",
        "links.html",
        "sidebar/scroll-end.html",
        "style.html",
    ],
}

# Additional templates that should be rendered to pages, maps page names to
# template names.
# html_additional_pages = {}
# html_additional_pages = {'index': 'index.html'}


# If false, no module index is generated.
html_domain_indices = True

# If false, no index is generated.
html_use_index = False

# If true, the index is split into individual pages for each letter.
# html_split_index = False

# If true, links to the reST sources are added to the pages.
html_show_sourcelink = False

# If true, "Created using Sphinx" is shown in the HTML footer. Default is True.
# html_show_sphinx = True

# If true, "(C) Copyright ..." is shown in the HTML footer. Default is True.
Repository: pytest-dev/pytest
File: testing/test_compat.py (Python)

Code before:

    assert is_generator(zap)
    assert not is_generator(foo)


def test_real_func_loop_limit() -> None:
    class Evil:
        def __init__(self):
            self.left = 1000

        def __repr__(self):
            return f"<Evil left={self.left}>"

        def __getattr__(self, attr):
            if not self.left:
                raise RuntimeError("it's over")  # pragma: no cover
            self.left -= 1
            return self

    evil = Evil()

    with pytest.raises(
        ValueError,
        match=(
            "could not find real function of <Evil left=900>\n"
            "stopped at <Evil left=900>"
        ),
    ):
        get_real_func(evil)


def test_get_real_func() -> None:
    """Check that get_real_func correctly unwraps decorators until reaching the real function"""

    def decorator(f):
        @wraps(f)
        def inner():
            pass  # pragma: no cover

        return inner

    def func():
        pass  # pragma: no cover

    wrapped_func = decorator(decorator(func))
    assert get_real_func(wrapped_func) is func

    wrapped_func2 = decorator(decorator(wrapped_func))
    assert get_real_func(wrapped_func2) is func

    # obtain the function up until the point a function was wrapped by pytest itself

Review comment (suggestion):
explanation: previously we were doing 2 get attributes but it's now replaced by isinstance check. So left is 100 more.

Code after:
        def __getattr__(self, attr):
            if not self.left:
                raise RuntimeError("it's over")  # pragma: no cover
            self.left -= 1
            return self

    evil = Evil()

    with pytest.raises(
        ValueError,
        match=("wrapper loop when unwrapping <Evil left=998>"),
    ):
        get_real_func(evil)


def test_get_real_func() -> None:
    """Check that get_real_func correctly unwraps decorators until reaching the real function"""

    def decorator(f):
        @wraps(f)
        def inner():
            pass  # pragma: no cover

        return inner

    def func():
        pass  # pragma: no cover

    wrapped_func = decorator(decorator(func))
    assert get_real_func(wrapped_func) is func

    wrapped_func2 = decorator(decorator(wrapped_func))
    assert get_real_func(wrapped_func2) is func

    # obtain the function up until the point a function was wrapped by pytest itself
    @pytest.fixture
    def wrapped_func3():
        pass  # pragma: no cover

    wrapped_func4 = decorator(wrapped_func3)
    assert get_real_func(wrapped_func4) is wrapped_func3._get_wrapped_function()


def test_get_real_func_partial() -> None:
    """Test get_real_func handles partial instances correctly"""

    def foo(x):
        return x

    assert get_real_func(foo) is foo
    assert get_real_func(partial(foo)) is foo