kou commented on code in PR #46530: URL: https://github.com/apache/arrow/pull/46530#discussion_r2110634441
########## dev/archery/archery/cli.py: ########## @@ -844,29 +868,28 @@ def integration(with_all=False, random_seed=12345, **args): implementations = ['cpp', 'csharp', 'java', 'js', 'go', 'nanoarrow', 'rust'] formats = ['ipc', 'flight', 'c_data'] - enabled_implementations = 0 + enabled_implementations : bool = False for lang in implementations: param = f'with_{lang}' if with_all: args[param] = with_all - enabled_implementations += args[param] + enabled_implementations = True - enabled_formats = 0 + enabled_formats : bool = False for fmt in formats: - param = f'run_{fmt}' - enabled_formats += args[param] + enabled_formats = True if gen_path: # XXX See GH-37575: this option is only used by the JS test suite # and might not be useful anymore. os.makedirs(gen_path, exist_ok=True) write_js_test_json(gen_path) else: - if enabled_formats == 0: + if enabled_formats == False: raise click.UsageError( "Need to enable at least one format to test " "(IPC, Flight, C Data Interface); try --help") - if enabled_implementations == 0: + if enabled_implementations == False: Review Comment: ```suggestion if not enabled_implementations: ``` ########## dev/archery/archery/cli.py: ########## @@ -745,6 +745,30 @@ def _set_default(opt, default): @click.option('--with-rust', type=bool, default=False, help='Include Rust in integration tests', envvar="ARCHERY_INTEGRATION_WITH_RUST") +@click.option('--with-external-library', type=click.Path(exists=True, file_okay=False, dir_okay=True, executable=False), + help='Include external library in integration tests', + envvar="ARCHERY_INTEGRATION_WITH_EXTERNAL_LIBRARY") +@click.option('--external-library-IPC-producer', type=bool, default=False, + help='Set external library as supporting producing IPC in integration tests', + envvar="ARCHERY_INTEGRATION_EXTERNAL_LIBRARY_IPC_PRODUCER") +@click.option('--external-library-IPC-consumer', type=bool, default=False, Review Comment: ```suggestion @click.option('--external-library-ipc-consumer', type=bool, default=False, ``` ########## dev/archery/archery/cli.py: ########## @@ -745,6 +745,30 @@ def _set_default(opt, default): @click.option('--with-rust', type=bool, default=False, help='Include Rust in integration tests', envvar="ARCHERY_INTEGRATION_WITH_RUST") +@click.option('--with-external-library', type=click.Path(exists=True, file_okay=False, dir_okay=True, executable=False), + help='Include external library in integration tests', + envvar="ARCHERY_INTEGRATION_WITH_EXTERNAL_LIBRARY") +@click.option('--external-library-IPC-producer', type=bool, default=False, Review Comment: ```suggestion @click.option('--external-library-ipc-producer', type=bool, default=False, ``` ########## dev/archery/archery/cli.py: ########## @@ -844,29 +868,28 @@ def integration(with_all=False, random_seed=12345, **args): implementations = ['cpp', 'csharp', 'java', 'js', 'go', 'nanoarrow', 'rust'] formats = ['ipc', 'flight', 'c_data'] - enabled_implementations = 0 + enabled_implementations : bool = False for lang in implementations: param = f'with_{lang}' if with_all: args[param] = with_all - enabled_implementations += args[param] + enabled_implementations = True Review Comment: Do we need this...? ```suggestion if args[param]: enabled_implementations = True ``` ########## dev/archery/archery/cli.py: ########## @@ -844,29 +868,28 @@ def integration(with_all=False, random_seed=12345, **args): implementations = ['cpp', 'csharp', 'java', 'js', 'go', 'nanoarrow', 'rust'] formats = ['ipc', 'flight', 'c_data'] - enabled_implementations = 0 + enabled_implementations : bool = False Review Comment: If we want to use `bool` for this, we may need to rename this something like `have_enabled_implementation`. ########## dev/archery/archery/cli.py: ########## @@ -844,29 +868,28 @@ def integration(with_all=False, random_seed=12345, **args): implementations = ['cpp', 'csharp', 'java', 'js', 'go', 'nanoarrow', 'rust'] formats = ['ipc', 'flight', 'c_data'] - enabled_implementations = 0 + enabled_implementations : bool = False for lang in implementations: param = f'with_{lang}' if with_all: args[param] = with_all - enabled_implementations += args[param] + enabled_implementations = True - enabled_formats = 0 + enabled_formats : bool = False for fmt in formats: - param = f'run_{fmt}' - enabled_formats += args[param] + enabled_formats = True if gen_path: # XXX See GH-37575: this option is only used by the JS test suite # and might not be useful anymore. os.makedirs(gen_path, exist_ok=True) write_js_test_json(gen_path) else: - if enabled_formats == 0: + if enabled_formats == False: Review Comment: ```suggestion if not enabled_formats: ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org