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")
[email protected]('--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")
[email protected]('--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")
[email protected]('--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")
[email protected]('--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")
[email protected]('--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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
