Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

style: update harness to eliminate I2041 flake8 errors #8960

Merged
merged 4 commits into from
Mar 28, 2024
Merged

style: update harness to eliminate I2041 flake8 errors #8960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8416430
style: update harness to eliminate I2041 flake8 errors
MikhailKardash Mar 5, 2024
388eb64
removed in anda's profiler pr
MikhailKardash Mar 27, 2024
bf8d210
cleanup some strings
MikhailKardash Mar 28, 2024
dd6a59c
weird rebase behavior
MikhailKardash Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 0 additions & 125 deletions harness/.flake8
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,132 +17,7 @@ exclude =
# be used by the importer.)
per-file-ignores =
__init__.py:F401,I2041
determined/cli/agent.py:I2041
determined/cli/checkpoint.py:I2041
determined/cli/cli.py:I2041
determined/cli/command.py:I2041
determined/cli/dev.py:I2041
determined/cli/experiment.py:I2041
determined/cli/job.py:I2041
determined/cli/master.py:I2041
determined/cli/model.py:I2041
determined/cli/notebook.py:I2041
determined/cli/ntsc.py:I2041
determined/cli/oauth.py:I2041
determined/cli/project.py:I2041
determined/cli/proxy.py:I2041
determined/cli/rbac.py:I2041
determined/cli/render.py:I2041
determined/cli/resource_pool.py:I2041
determined/cli/resources.py:I2041
determined/cli/shell.py:I2041
determined/cli/sso.py:I2041
determined/cli/task.py:I2041
determined/cli/template.py:I2041
determined/cli/tensorboard.py:I2041
determined/cli/top_arg_descriptions.py:I2041
determined/cli/trial.py:I2041
determined/cli/tunnel.py:I2041
determined/cli/user.py:I2041
determined/cli/user_groups.py:I2041
determined/cli/version.py:I2041
determined/cli/workspace.py:I2041
determined/common/declarative_argparse.py:I2041
determined/common/experimental/session.py:I2041
determined/common/storage/azure_client.py:I2041
determined/common/storage/boto3_credential_manager.py:I2041
determined/common/storage/gcs.py:I2041
determined/deploy/aws/aws.py:I2041
determined/deploy/aws/cli.py:I2041
determined/deploy/aws/deployment_types/base.py:I2041
determined/deploy/aws/deployment_types/govcloud.py:I2041
determined/deploy/aws/deployment_types/secure.py:I2041
determined/deploy/aws/gen_vcpu_mapping.py:I2041
determined/deploy/aws/master_config_inject.py:I2041
determined/deploy/aws/preflight.py:I2041
determined/deploy/cli.py:I2041
determined/deploy/gcp/cli.py:I2041
determined/deploy/gcp/gcp.py:I2041
determined/deploy/gcp/preflight.py:I2041
determined/deploy/gke/cli.py:I2041
determined/deploy/healthcheck.py:I2041
determined/deploy/local/cli.py:I2041
determined/deploy/local/cluster_utils.py:I2041
determined/deploy/local/preflight.py:I2041
determined/experimental/client.py:I2041
determined/experimental/unmanaged.py:I2041
determined/keras/_load.py:I2041
determined/keras/_tf_keras_context.py:I2041
determined/keras/_tf_keras_trial.py:I2041
determined/launch/deepspeed.py:I2041
determined/launch/horovod.py:I2041
determined/lightning/experimental.py:I2041
determined/profiler.py:I2041
determined/pytorch/_data.py:I2041
determined/pytorch/_pytorch_context.py:I2041
determined/pytorch/_pytorch_trial.py:I2041
determined/pytorch/_trainer.py:I2041
determined/pytorch/deepspeed/_deepspeed_context.py:I2041
determined/pytorch/deepspeed/_mpu.py:I2041
determined/pytorch/dsat/_dsat_search_method.py:I2041
determined/pytorch/dsat/_run_dsat.py:I2041
determined/pytorch/dsat/_utils.py:I2041
determined/searcher/_remote_search_runner.py:I2041
determined/searcher/_search_method.py:I2041
determined/searcher/_search_runner.py:I2041
determined/tensorboard/base.py:I2041
determined/tensorboard/build.py:I2041
determined/tensorboard/fetchers/azure.py:I2041
determined/tensorboard/fetchers/gcs.py:I2041
determined/tensorboard/fetchers/s3.py:I2041
determined/tensorboard/fetchers/shared.py:I2041
determined/tensorboard/gcs.py:I2041
determined/tensorboard/metric_writers/pytorch.py:I2041
determined/tensorboard/metric_writers/tensorflow.py:I2041
determined/tensorboard/s3.py:I2041
determined/transformers/_hf_callback.py:I2041
determined/transformers/experimental.py:I2041
determined/workload.py:I2041
tests/checkpoints/test_checkpoint.py:I2041
tests/cli/filetree.py:I2041
tests/cli/test_auth.py:I2041
tests/cli/test_cli.py:I2041
tests/cli/test_experiment.py:I2041
tests/common/api_server.py:I2041
tests/common/test_api_server.py:I2041
tests/common/test_multimaster.py:I2041
tests/confdir.py:I2041
tests/conftest.py:I2041
tests/custom_search_mocks.py:I2041
tests/determined/common/experimental/test_determined.py:I2041
tests/experiment/fixtures/ancient_keras_ckpt.py:I2041
tests/experiment/fixtures/keras_no_op/model_def.py:I2041
tests/experiment/fixtures/keras_tf2_disabled_no_op/model_def.py:I2041
tests/experiment/fixtures/pytorch_amp/apex_amp_model_def.py:I2041
tests/experiment/fixtures/pytorch_amp/auto_amp_model_def.py:I2041
tests/experiment/fixtures/pytorch_amp/layers.py:I2041
tests/experiment/fixtures/pytorch_amp/manual_amp_model_def.py:I2041
tests/experiment/fixtures/pytorch_amp/model_def.py:I2041
tests/experiment/fixtures/tf_keras_one_var_model.py:I2041
tests/experiment/fixtures/tf_keras_xor_model.py:I2041
tests/experiment/keras/test_keras_data.py:I2041
tests/experiment/keras/test_tf_keras_trial.py:I2041
tests/experiment/pytorch/test_deepspeed_autotuning.py:I2041
tests/experiment/pytorch/test_pytorch_data.py:I2041
tests/experiment/pytorch/test_reducer.py:I2041
tests/experiment/tensorflow/test_util.py:I2041
tests/experiment/test_local.py:I2041
tests/experiment/utils.py:I2041
tests/filetree.py:I2041
tests/launch/test_deepspeed.py:I2041
tests/search_methods.py:I2041
tests/storage/test_azure.py:I2041
tests/storage/test_gcs.py:I2041
tests/storage/test_s3.py:I2041
tests/storage/test_shared_fs.py:I2041
tests/tensorboard/test_util.py:I2041
tests/test_custom_searcher.py:I2041
tests/test_gc_harness.py:I2041

# Explanations for ignored error codes:
# - D1* (no missing docstrings): too much effort to start enforcing
Expand Down
35 changes: 15 additions & 20 deletions harness/determined/cli/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,22 @@
wait_ntsc_ready,
warn,
)
from determined.cli import (
agent,
checkpoint,
cli,
ntsc,
command,
experiment,
master,
model,
notebook,
project,
rbac,
render,
resources,
shell,
template,
tensorboard,
trial,
user,
workspace,
from determined.cli._declarative_argparse import (
Arg,
ArgsDescription,
ArgGroup,
BoolOptArg,
Cmd,
Group,
add_args,
deprecation_warning,
help_func,
generate_aliases,
make_prefixes,
string_to_bool,
wrap_func,
)
from determined.cli.errors import CliError

from determined.common.api import certs as _certs
from typing import Optional as _Optional
Expand Down
22 changes: 11 additions & 11 deletions ...determined/common/declarative_argparse.py → ...s/determined/cli/_declarative_argparse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import argparse
import functools
import itertools
from argparse import SUPPRESS, ArgumentDefaultsHelpFormatter, ArgumentParser, Namespace
from typing import Any, Callable, List, NamedTuple, Optional, Tuple, Union, cast

from termcolor import colored
import termcolor

from determined.common import util

Expand Down Expand Up @@ -40,7 +40,7 @@ def generate_aliases(spec: str) -> Tuple[str, List[str]]:

def deprecation_warning(message: str, color: bool = True) -> str:
msg = f"DEPRECATED: {message}"
return colored(msg, "yellow") if color else msg
return termcolor.colored(msg, "yellow") if color else msg


# Classes used to represent the structure of an argument parser setup; these
Expand Down Expand Up @@ -126,16 +126,16 @@ class BoolOptArg(NamedTuple):
false_help: Optional[str] = None


def wrap_func(parser: ArgumentParser, func: Callable) -> Callable:
def wrap_func(parser: argparse.ArgumentParser, func: Callable) -> Callable:
@functools.wraps(func)
def wrapper(args: Namespace) -> Any:
def wrapper(args: argparse.Namespace) -> Any:
args.func = func
return func(parser.parse_args([], args))

return wrapper


def help_func(parser: ArgumentParser) -> Callable:
def help_func(parser: argparse.ArgumentParser) -> Callable:
"""
Return a function that prints help for the given parser. Using this doesn't
exit during the call to to `parse_args` itself, which would be ideal, but
Expand All @@ -144,13 +144,13 @@ def help_func(parser: ArgumentParser) -> Callable:
least.
"""

def inner_func(args: Namespace) -> Any:
def inner_func(args: argparse.Namespace) -> Any:
parser.print_help()

return inner_func


def add_args(parser: ArgumentParser, description: ArgsDescription, depth: int = 0) -> None:
def add_args(parser: argparse.ArgumentParser, description: ArgsDescription, depth: int = 0) -> None:
"""
Populate the given parser with arguments, as specified by the
description. The description is a list of Arg, Cmd, and Group objects.
Expand Down Expand Up @@ -186,9 +186,9 @@ def description_sort_key(desc: Any) -> str:

subparser_kwargs = {
"aliases": aliases,
"formatter_class": ArgumentDefaultsHelpFormatter,
"formatter_class": argparse.ArgumentDefaultsHelpFormatter,
}
if thing.help_str != SUPPRESS:
if thing.help_str != argparse.SUPPRESS:
if thing.deprecation_message:
thing.help_str += " " + deprecation_warning(
thing.deprecation_message, color=False
Expand Down Expand Up @@ -235,7 +235,7 @@ def description_sort_key(desc: Any) -> str:
dest=thing.dest,
action="store_false",
help=thing.false_help,
default=SUPPRESS,
default=argparse.SUPPRESS,
)

# If there are any subcommands but none claimed the default action, make
Expand Down
25 changes: 14 additions & 11 deletions harness/determined/cli/_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,30 @@
import termcolor

from determined import cli

# avoid circular import
from determined.cli import _declarative_argparse as detparse
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I were making this PR I'd probably do as cli here so the usage looks the same as "standard" one, and then Ryan will look at it and say it's an awful idea and I'd have to change it to something like this. anyhow, disregard this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's a bit awkward. Another solution would be to refactor parts of this file into _declarative_argparse.py.

from determined.cli import errors, render
from determined.common import api, declarative_argparse
from determined.common import api
from determined.common.api import authentication, bindings

output_format_args: Dict[str, declarative_argparse.Arg] = {
"json": declarative_argparse.Arg(
output_format_args: Dict[str, detparse.Arg] = {
"json": detparse.Arg(
"--json",
action="store_true",
help="Output in JSON format",
),
"yaml": declarative_argparse.Arg(
"yaml": detparse.Arg(
"--yaml",
action="store_true",
help="Output in YAML format",
),
"csv": declarative_argparse.Arg(
"csv": detparse.Arg(
"--csv",
action="store_true",
help="Output in CSV format",
),
"table": declarative_argparse.Arg(
"table": detparse.Arg(
"--table",
action="store_true",
help="Output in table format",
Expand All @@ -42,24 +45,24 @@ def make_pagination_args(
offset: int = 0,
pages: api.PageOpts = api.PageOpts.all,
supports_reverse: bool = False,
) -> List[declarative_argparse.Arg]:
) -> List[detparse.Arg]:
if pages not in PAGE_CHOICES:
raise NotImplementedError

res = [
declarative_argparse.Arg(
detparse.Arg(
"--limit",
type=int,
default=limit,
help="Maximum items per page of results",
),
declarative_argparse.Arg(
detparse.Arg(
"--offset",
type=int,
default=offset,
help="Number of items to skip before starting page of results",
),
declarative_argparse.Arg(
detparse.Arg(
"--pages",
type=api.PageOpts,
choices=PAGE_CHOICES,
Expand All @@ -70,7 +73,7 @@ def make_pagination_args(

if supports_reverse:
res += [
declarative_argparse.Arg(
detparse.Arg(
"--reverse",
default=False,
action="store_true",
Expand Down
Loading
Loading