ronantakizawa/codereview-bench huggingface dataset:
Repository: jina-ai/jina
File: jina/parsers/logging.py (Python)

Code before:
"""Extensive argparser module for logging"""


def mixin_suppress_root_logging_parser(parser):
    parser.add_argument(
        '--suppress_root_logging',
        action='store_true',
        default=False,
        help='If set, then no root handlers will be suppressed from logging.',
    )


Review comment (suggestion):
CLI args are separated by `-` instead of underscore. The python variable name will be available with underscores.

Code after:
"""Extensive argparser module for logging"""


def mixin_suppress_root_logging_parser(parser):
    """Mixing in arguments required by every module into the given parser.
    This parser extends logging-related arguments.
    :param parser: the parser instance to which we add arguments
    """
    parser.add_argument(
        '--suppress-root-logging',
        action='store_true',
        default=False,
        help='If set, then no root handlers will be suppressed from logging.',
    )
Repository: jina-ai/jina
File: jina_cli/api.py (Python)

Code before:
from typing import TYPE_CHECKING

from jina.parsers.helper import _update_gateway_args

if TYPE_CHECKING:
    from argparse import Namespace


def deployment(args: 'Namespace'):
    """
    Start a Deployment

    :param args: arguments coming from the CLI.
    """
    from jina.orchestrate.deployments import Deployment

    if args.uses:
        f = Deployment.load_config(args.uses)
        with f:
            f.block()
    else:
        raise ValueError('start a Deployment from CLI requires a valid `--uses`')


def pod(args: 'Namespace'):
    """
    Start a Pod

    :param args: arguments coming from the CLI.
    """
    from jina.orchestrate.pods.factory import PodFactory

    try:
        with PodFactory.build_pod(args) as p:
            p.join()
    except KeyboardInterrupt:
        pass


def executor_native(args: 'Namespace'):
    """
    Starts an Executor in a WorkerRuntime

    :param args: arguments coming from the CLI.
    """

Review comment (suggestion):
```suggestion
        dep = Deployment.load_config(args.uses)
        with f:
            dep.block()
```

Code after:
from typing import TYPE_CHECKING

from jina.parsers.helper import _update_gateway_args

if TYPE_CHECKING:
    from argparse import Namespace


def deployment(args: 'Namespace'):
    """
    Start a Deployment

    :param args: arguments coming from the CLI.
    """
    from jina.orchestrate.deployments import Deployment

    if args.uses:
        dep = Deployment.load_config(args.uses)
        with dep:
            dep.block()
    else:
        raise ValueError('starting a Deployment from CLI requires a valid `--uses`')


def pod(args: 'Namespace'):
    """
    Start a Pod

    :param args: arguments coming from the CLI.
    """
    from jina.orchestrate.pods.factory import PodFactory

    try:
        with PodFactory.build_pod(args) as p:
            p.join()
    except KeyboardInterrupt:
        pass


def executor_native(args: 'Namespace'):
    """
    Starts an Executor in a WorkerRuntime

    :param args: arguments coming from the CLI.
    """
Repository: jina-ai/jina
File: jina_cli/api.py (Python)

Code before:
from typing import TYPE_CHECKING

from jina.parsers.helper import _update_gateway_args

if TYPE_CHECKING:
    from argparse import Namespace


def deployment(args: 'Namespace'):
    """
    Start a Deployment

    :param args: arguments coming from the CLI.
    """
    from jina.orchestrate.deployments import Deployment

    if args.uses:
        f = Deployment.load_config(args.uses)
        with f:
            f.block()
    else:
        raise ValueError('start a Deployment from CLI requires a valid `--uses`')


def pod(args: 'Namespace'):
    """
    Start a Pod

    :param args: arguments coming from the CLI.
    """
    from jina.orchestrate.pods.factory import PodFactory

    try:
        with PodFactory.build_pod(args) as p:
            p.join()
    except KeyboardInterrupt:
        pass


def executor_native(args: 'Namespace'):
    """
    Starts an Executor in a WorkerRuntime

    :param args: arguments coming from the CLI.
    """

    if args.runtime_cls == 'WorkerRuntime':

Review comment (suggestion):
```suggestion
        raise ValueError('starting a Deployment from CLI requires a valid `--uses`')
```

Code after:
from typing import TYPE_CHECKING

from jina.parsers.helper import _update_gateway_args

if TYPE_CHECKING:
    from argparse import Namespace


def deployment(args: 'Namespace'):
    """
    Start a Deployment

    :param args: arguments coming from the CLI.
    """
    from jina.orchestrate.deployments import Deployment

    if args.uses:
        dep = Deployment.load_config(args.uses)
        with dep:
            dep.block()
    else:
        raise ValueError('starting a Deployment from CLI requires a valid `--uses`')


def pod(args: 'Namespace'):
    """
    Start a Pod

    :param args: arguments coming from the CLI.
    """
    from jina.orchestrate.pods.factory import PodFactory

    try:
        with PodFactory.build_pod(args) as p:
            p.join()
    except KeyboardInterrupt:
        pass


def executor_native(args: 'Namespace'):
    """
    Starts an Executor in a WorkerRuntime

    :param args: arguments coming from the CLI.
    """

    if args.runtime_cls == 'WorkerRuntime':
Repository: jina-ai/jina
File: jina_cli/api.py (Python)

Code before:

def export(args: 'Namespace'):
    """
    Export the API

    :param args: arguments coming from the CLI.
    """
    from jina import exporter

    getattr(exporter, f'export_{args.export.replace("-", "_")}')(args)


def flow(args: 'Namespace'):
    """
    Start a Flow from a YAML file or a docker image

    :param args: arguments coming from the CLI.
    """
    from jina import Flow

    if args.uses:
        f = Flow.load_config(args.uses)
        with f:
            f.block()
    else:
        raise ValueError('start a Flow from CLI requires a valid `--uses`')


def hub(args: 'Namespace'):
    """
    Start a hub builder for push, pull
    :param args: arguments coming from the CLI.
    """
    from hubble.executor.hubio import HubIO

    getattr(HubIO(args), args.hub_cli)()


def new(args: 'Namespace'):
    """
    Create a new jina project
    :param args:  arguments coming from the CLI.
    """
    import os
    import shutil

    from jina.constants import __resources_path__

    shutil.copytree(
        os.path.join(__resources_path__, 'project-template'), os.path.abspath(args.name)
    )

Review comment (suggestion):
```suggestion
        raise ValueError('starting a Flow from CLI requires a valid `--uses`')
```

Code after:

def export(args: 'Namespace'):
    """
    Export the API

    :param args: arguments coming from the CLI.
    """
    from jina import exporter

    getattr(exporter, f'export_{args.export.replace("-", "_")}')(args)


def flow(args: 'Namespace'):
    """
    Start a Flow from a YAML file or a docker image

    :param args: arguments coming from the CLI.
    """
    from jina import Flow

    if args.uses:
        f = Flow.load_config(args.uses)
        with f:
            f.block()
    else:
        raise ValueError('starting a Flow from CLI requires a valid `--uses`')


def hub(args: 'Namespace'):
    """
    Start a hub builder for push, pull
    :param args: arguments coming from the CLI.
    """
    from hubble.executor.hubio import HubIO

    getattr(HubIO(args), args.hub_cli)()


def new(args: 'Namespace'):
    """
    Create a new jina project
    :param args:  arguments coming from the CLI.
    """
    import os
    import shutil

    from jina.constants import __resources_path__

    shutil.copytree(
        os.path.join(__resources_path__, 'project-template'), os.path.abspath(args.name)
    )
Repository: jina-ai/jina
File: tests/integration/deployments/test_deployment.py (Python)

Code before:

def _create_regular_deployment(
    port,
    name='',
    executor=None,
    uses_before=False,
    uses_after=False,
    polling=PollingType.ANY,
    shards=None,
    replicas=None,
):
    args = set_deployment_parser().parse_args(['--port', str(port)])
    args.name = name
    if shards:
        args.shards = shards
    if replicas:
        args.replicas = replicas
    args.polling = polling
    if executor:
        args.uses = executor if executor else 'NameChangeExecutor'
    if uses_after:
        args.uses_after = executor if executor else 'NameChangeExecutor'
    if uses_before:
        args.uses_before = executor if executor else 'NameChangeExecutor'
    return Deployment(args, include_gateway=False)


def _create_gateway_deployment(
    graph_description, deployments_addresses, deployments_metadata, port
):
    return Deployment(
        set_gateway_parser().parse_args(
            [
                '--graph-description',
                graph_description,
                '--deployments-addresses',
                deployments_addresses,
                '--deployments-metadata',
                deployments_metadata,
                '--port',
                str(port),
            ]
        ),
        include_gateway=False,
    )


async def async_inputs():
    for _ in range(20):
        yield Document(text='client0-Request')


Review comment (suggestion):
```suggestion
    return Deployment(args, include_gateway=False)
```

Code after:

def _create_regular_deployment(
    port,
    name='',
    executor=None,
    uses_before=False,
    uses_after=False,
    polling=PollingType.ANY,
    shards=None,
    replicas=None,
):
    args = set_deployment_parser().parse_args(['--port', str(port)])
    args.name = name
    if shards:
        args.shards = shards
    if replicas:
        args.replicas = replicas
    args.polling = polling
    if executor:
        args.uses = executor if executor else 'NameChangeExecutor'
    if uses_after:
        args.uses_after = executor if executor else 'NameChangeExecutor'
    if uses_before:
        args.uses_before = executor if executor else 'NameChangeExecutor'
    # is this needed ?
    return Deployment(args, include_gateway=False, noblock_on_start=False)


def _create_gateway_deployment(
    graph_description, deployments_addresses, deployments_metadata, port
):
    return Deployment(
        set_gateway_parser().parse_args(
            [
                '--graph-description',
                graph_description,
                '--deployments-addresses',
                deployments_addresses,
                '--deployments-metadata',
                deployments_metadata,
                '--port',
                str(port),
            ]
        ),
        include_gateway=False,
    )


async def async_inputs():
    for _ in range(20):
        yield Document(text='client0-Request')
Repository: jina-ai/jina
File: tests/conftest.py (Python)

Code before:
    )


@pytest.fixture(scope='function')
def port_generator():
    generated_ports = set()

    def random_port():
        port = helper.random_port()
        while port in generated_ports:
            port = helper.random_port()
        generated_ports.add(port)
        return port

    return random_port


@pytest.fixture(autouse=True)
def test_log_level(monkeypatch):
    monkeypatch.setenv('JINA_LOG_LEVEL', 'DEBUG')


@pytest.fixture(autouse=True)
def test_grpc_fork_support_false(monkeypatch):
    monkeypatch.setenv('GRPC_ENABLE_FORK_SUPPORT', 'true')
    monkeypatch.setenv('GRPC_VERBOSITY', 'DEBUG')



@pytest.fixture(autouse=True)
def test_timeout_ctrl_time(monkeypatch):
    monkeypatch.setenv('JINA_DEFAULT_TIMEOUT_CTRL', '500')


@pytest.fixture(autouse=True)
def test_disable_telemetry(monkeypatch):
    monkeypatch.setenv('JINA_OPTOUT_TELEMETRY', 'True')


@pytest.fixture(autouse=True)
def tmpfile(tmpdir):
    tmpfile = f'jina_test_{next(tempfile._get_candidate_names())}.db'
    return tmpdir / tmpfile


@pytest.fixture(scope='session')
def event_loop(request):
    """
    Valid only for `pytest.mark.asyncio` tests
    """
    loop = asyncio.get_event_loop_policy().new_event_loop()

Review comment (suggestion):
```suggestion
    monkeypatch.setenv('GRPC_POLL_STRATEGY' , 'poll')
```

Code after:
    )


@pytest.fixture(scope='function')
def port_generator():
    generated_ports = set()

    def random_port():
        port = helper.random_port()
        while port in generated_ports:
            port = helper.random_port()
        generated_ports.add(port)
        return port

    return random_port


@pytest.fixture(autouse=True)
def test_log_level(monkeypatch):
    monkeypatch.setenv('JINA_LOG_LEVEL', 'DEBUG')


@pytest.fixture(autouse=True)
def test_grpc_fork_support_false(monkeypatch):
    monkeypatch.setenv('GRPC_ENABLE_FORK_SUPPORT', 'true')



@pytest.fixture(autouse=True)
def test_timeout_ctrl_time(monkeypatch):
    monkeypatch.setenv('JINA_DEFAULT_TIMEOUT_CTRL', '500')


@pytest.fixture(autouse=True)
def test_disable_telemetry(monkeypatch):
    monkeypatch.setenv('JINA_OPTOUT_TELEMETRY', 'True')


@pytest.fixture(autouse=True)
def tmpfile(tmpdir):
    tmpfile = f'jina_test_{next(tempfile._get_candidate_names())}.db'
    return tmpdir / tmpfile


@pytest.fixture(scope='session')
def event_loop(request):
    """
    Valid only for `pytest.mark.asyncio` tests
    """
    loop = asyncio.get_event_loop_policy().new_event_loop()
    yield loop
Repository: jina-ai/jina
File: docs/conf.py (Python)

Code before:
import os
import re
import sys
from os import path

version = ''
# Centralize environment variables
JINA_VERSION = os.environ.get('JINA_VERSION')
SPHINX_MULTIVERSION_VERSION = os.environ.get('SPHINX_MULTIVERSION_VERSION', version)

sys.path.insert(0, path.abspath('..'))

project = 'Jina'
slug = re.sub(r'\W+', '-', project.lower())
author = 'Jina AI'
copyright = 'Jina AI Limited. All rights reserved.'
source_suffix = ['.rst', '.md']
master_doc = 'index'
language = 'en'
repo_dir = '../'

try:
    if not JINA_VERSION:
        pkg_name = 'jina'
        libinfo_py = path.join(repo_dir, pkg_name, '__init__.py')
        libinfo_content = open(libinfo_py, 'r', encoding='utf-8').readlines()
        version_line = [
            l.strip() for l in libinfo_content if l.startswith('__version__')
        ][0]
        exec(version_line)
    else:
        __version__ = JINA_VERSION
except FileNotFoundError:
    __version__ = '0.0.0'

Review comment (suggestion):
this is not what is intended. See that `version` is used later, and thus this reading should be placed after the `version` var initialization

Code after:
import os
import re
import sys
from os import path


# Centralize environment variables
JINA_VERSION = os.environ.get('JINA_VERSION', None)

sys.path.insert(0, path.abspath('..'))

project = 'Jina'
slug = re.sub(r'\W+', '-', project.lower())
author = 'Jina AI'
copyright = 'Jina AI Limited. All rights reserved.'
source_suffix = ['.rst', '.md']
master_doc = 'index'
language = 'en'
repo_dir = '../'

try:
    if not JINA_VERSION:
        pkg_name = 'jina'
        libinfo_py = path.join(repo_dir, pkg_name, '__init__.py')
        libinfo_content = open(libinfo_py, 'r', encoding='utf-8').readlines()
        version_line = [
            l.strip() for l in libinfo_content if l.startswith('__version__')
        ][0]
        exec(version_line)
    else:
        __version__ = JINA_VERSION
except FileNotFoundError:
    __version__ = '0.0.0'
Repository: jina-ai/jina
File: docs/conf.py (Python)

Code before:
import os
import re
import sys
from os import path

version = ''
# Centralize environment variables
JINA_VERSION = os.environ.get('JINA_VERSION')
SPHINX_MULTIVERSION_VERSION = os.environ.get('SPHINX_MULTIVERSION_VERSION', version)

sys.path.insert(0, path.abspath('..'))

project = 'Jina'
slug = re.sub(r'\W+', '-', project.lower())
author = 'Jina AI'
copyright = 'Jina AI Limited. All rights reserved.'
source_suffix = ['.rst', '.md']
master_doc = 'index'
language = 'en'
repo_dir = '../'

try:
    if not JINA_VERSION:
        pkg_name = 'jina'
        libinfo_py = path.join(repo_dir, pkg_name, '__init__.py')
        libinfo_content = open(libinfo_py, 'r', encoding='utf-8').readlines()
        version_line = [
            l.strip() for l in libinfo_content if l.startswith('__version__')
        ][0]
        exec(version_line)
    else:
        __version__ = JINA_VERSION
except FileNotFoundError:

Review comment (suggestion):
```suggestion
JINA_VERSION = os.environ.get('JINA_VERSION', None)
```

Code after:
import os
import re
import sys
from os import path


# Centralize environment variables
JINA_VERSION = os.environ.get('JINA_VERSION', None)

sys.path.insert(0, path.abspath('..'))

project = 'Jina'
slug = re.sub(r'\W+', '-', project.lower())
author = 'Jina AI'
copyright = 'Jina AI Limited. All rights reserved.'
source_suffix = ['.rst', '.md']
master_doc = 'index'
language = 'en'
repo_dir = '../'

try:
    if not JINA_VERSION:
        pkg_name = 'jina'
        libinfo_py = path.join(repo_dir, pkg_name, '__init__.py')
        libinfo_content = open(libinfo_py, 'r', encoding='utf-8').readlines()
        version_line = [
            l.strip() for l in libinfo_content if l.startswith('__version__')
        ][0]
        exec(version_line)
    else:
        __version__ = JINA_VERSION
except FileNotFoundError:
    __version__ = '0.0.0'
Repository: jquery/jquery
File: src/traversing.js (JavaScript)

Code before:

		return this.filter(function() {
			for ( i = 0; i < len; i++ ) {
				if ( jQuery.contains( this, targets[i] ) ) {
					return true;
				}
			}
		});
	},

	not: function( selector ) {
		return this.pushStack( winnow(this, selector, false) );
	},

	filter: function( selector ) {
		return this.pushStack( winnow(this, selector, true) );
	},

	is: function( selector ) {
		return !!selector && (
			typeof selector === "string" ?
				// If this is a positional/relative selector, check membership in the returned set
				// so $("p:first").is("p:last") won't return true for a doc with two "p".
				rneedsContext.test( selector ) ?
					jQuery( selector, this.context ).index( this[0] ) >= 0 :
					jQuery.filter( selector, this ).length > 0 :
				this.filter( selector ).length > 0 );
	},

	closest: function( selectors, context ) {
		var cur,
			i = 0,
			l = this.length,
			ret = [],
			pos = rneedsContext.test( selectors ) || typeof selectors !== "string" ?
				jQuery( selectors, context || this.context ) :
				0;

		for ( ; i < l; i++ ) {
			cur = this[i];

			while ( cur && cur.ownerDocument && cur !== context && cur.nodeType !== 11 ) {
				if ( pos ? pos.index(cur) > -1 : jQuery.find.matchesSelector(cur, selectors) ) {
					ret.push( cur );
					break;
				}
				cur = cur.parentNode;
			}
		}

		return this.pushStack( ret.length > 1 ? jQuery.unique( ret ) : ret );

Review comment (suggestion):
This is one of those places where we make the exception, ie. `[0]` is ok


Code after:

		return this.filter(function() {
			var i = 0;
			for ( ; i < l; i++ ) {
				if ( jQuery.contains( this, targets[i] ) ) {
					return true;
				}
			}
		});
	},

	not: function( selector ) {
		return this.pushStack( winnow(this, selector, false) );
	},

	filter: function( selector ) {
		return this.pushStack( winnow(this, selector, true) );
	},

	is: function( selector ) {
		return !!selector && (
			typeof selector === "string" ?
				// If this is a positional/relative selector, check membership in the returned set
				// so $("p:first").is("p:last") won't return true for a doc with two "p".
				rneedsContext.test( selector ) ?
					jQuery( selector, this.context ).index( this[ 0 ] ) >= 0 :
					jQuery.filter( selector, this ).length > 0 :
				this.filter( selector ).length > 0 );
	},

	closest: function( selectors, context ) {
		var cur,
			i = 0,
			l = this.length,
			matched = [],
			pos = ( rneedsContext.test( selectors ) || typeof selectors !== "string" ) ?
				jQuery( selectors, context || this.context ) :
				0;

		for ( ; i < l; i++ ) {
			cur = this[ i ];

			while ( cur && cur.ownerDocument && cur !== context ) {
				if ( pos ? pos.index( cur ) > -1 : jQuery.find.matchesSelector( cur, selectors ) ) {
					matched.push( cur );
					break;
				}
				cur = cur.parentElement;
			}
		}
Repository: jquery/jquery
File: src/core.js (JavaScript)

Code before:
	},

	isArray: Array.isArray || function( obj ) {
		return jQuery.type(obj) === "array";
	},

	isWindow: function( obj ) {
		return obj != null && obj == obj.window;
	},

	isNumeric: function( obj ) {
		return !isNaN( parseFloat(obj) ) && isFinite( obj );
	},

	type: function( obj ) {
		if ( obj == null ) {
			return String( obj );
		}
		return typeof obj === "object" || typeof obj === "function" ?
			class2type[ core_toString.call(obj) ] || "object" :
			typeof obj;
	},

	isPlainObject: function( obj ) {
		// Must be an Object.
		// Because of IE, we also have to check the presence of the constructor property.
		// Make sure that DOM nodes and window objects don't pass through, as well
		if ( !obj || jQuery.type(obj) !== "object" || obj.nodeType || jQuery.isWindow( obj ) ) {
			return false;
		}

		try {
			// Not own constructor property must be Object
			if ( obj.constructor &&
				!core_hasOwn.call(obj, "constructor") &&
				!core_hasOwn.call(obj.constructor.prototype, "isPrototypeOf") ) {
				return false;
			}
		} catch ( e ) {
			// IE8,9 Will throw exceptions on certain host objects #9897
			return false;
		}

		// Own properties are enumerated firstly, so to speed up,
		// if last one is own, then all properties are own.

		var key;
		for ( key in obj ) {}

		return key === undefined || core_hasOwn.call( obj, key );
	},

Review comment (suggestion):
<blockquote>Because of IE, we also have to check the presence of the constructor property.</blockquote>

I'm not sure which line this corresponded to, the closest line I could fine is "`if ( obj.constructor &&`" but that line was not removed.


Code after:
	isPlainObject: function( obj ) {
		// Not plain objects:
		// - Any object or value whose internal [[Class]] property is not "[object Object]"
		// - DOM nodes
		// - window
		if ( jQuery.type( obj ) !== "object" || obj.nodeType || jQuery.isWindow( obj ) ) {
			return false;
		}

		// Support: Firefox >16
		// The try/catch supresses exceptions thrown when attempting to access
		// the "constructor" property of certain host objects, ie. |window.location|
		try {
			if ( obj.constructor &&
					!core_hasOwn.call( obj.constructor.prototype, "isPrototypeOf" ) ) {
				return false;
			}
		} catch ( e ) {
			return false;
		}

		// If the function hasn't returned already, we're confident that
		// |obj| is a plain object, created by {} or constructed with new Object
		return true;
	},

	isEmptyObject: function( obj ) {
		var name;
		for ( name in obj ) {
			return false;
		}
		return true;
	},

	error: function( msg ) {
		throw new Error( msg );
	},

	// data: string of html
	// context (optional): If specified, the fragment will be created in this context, defaults to document
	// keepScripts (optional): If true, will include scripts passed in the html string
	parseHTML: function( data, context, keepScripts ) {
		if ( !data || typeof data !== "string" ) {
			return null;
		}
		if ( typeof context === "boolean" ) {
			keepScripts = context;
			context = false;
		}
		context = context || document;