From 0d236db63370425a892db0a5a27c260b2a439105 Mon Sep 17 00:00:00 2001 From: opendansor Date: Thu, 29 Aug 2024 11:24:29 -0700 Subject: [PATCH 1/9] Child Hotkeys netuid Refactor --- bittensor/commands/stake.py | 59 ++++++++++++------- .../subcommands/stake/test_childkeys.py | 8 +-- 2 files changed, 43 insertions(+), 24 deletions(-) diff --git a/bittensor/commands/stake.py b/bittensor/commands/stake.py index 132529a13..24868d048 100644 --- a/bittensor/commands/stake.py +++ b/bittensor/commands/stake.py @@ -44,19 +44,25 @@ def get_netuid( - cli: "bittensor.cli", subtensor: "bittensor.subtensor" + cli: "bittensor.cli", subtensor: "bittensor.subtensor", prompt: bool = True ) -> Tuple[bool, int]: """Retrieve and validate the netuid from the user or configuration.""" console = Console() - if not cli.config.is_set("netuid"): - try: - cli.config.netuid = int(Prompt.ask("Enter netuid")) - except ValueError: - console.print( - "[red]Invalid input. Please enter a valid integer for netuid.[/red]" - ) - return False, -1 + if not cli.config.is_set("netuid") and prompt: + cli.config.netuid = Prompt.ask("Enter netuid") + try: + cli.config.netuid = int(cli.config.netuid) + except ValueError: + console.print( + "[red]Invalid input. Please enter a valid integer for netuid.[/red]" + ) + return False, -1 netuid = cli.config.netuid + if netuid < 0 or netuid > 2**32 - 1: + console.print( + "[red]Invalid input. Please enter a valid integer for netuid in subnet range.[/red]" + ) + return False, -1 if not subtensor.subnet_exists(netuid=netuid): console.print( "[red]Network with netuid {} does not exist. Please try again.[/red]".format( @@ -1136,10 +1142,27 @@ def _run(cli: "bittensor.cli", subtensor: "bittensor.subtensor"): wallet = bittensor.wallet(config=cli.config) # check all - if not cli.config.is_set("all"): - exists, netuid = get_netuid(cli, subtensor) - if not exists: - return + if cli.config.is_set("all"): + cli.config.netuid = None + cli.config.all = True + elif cli.config.is_set("netuid"): + if cli.config.netuid == "all": + cli.config.all = True + else: + cli.config.netuid = int(cli.config.netuid) + exists, netuid = get_netuid(cli, subtensor) + if not exists: + return + else: + netuid_input = Prompt.ask("Enter netuid or 'all'", default="all") + if netuid_input == "all": + cli.config.netuid = None + cli.config.all = True + else: + cli.config.netuid = int(netuid_input) + exists, netuid = get_netuid(cli, subtensor, False) + if not exists: + return # get parent hotkey hotkey = get_hotkey(wallet, cli.config) @@ -1148,11 +1171,7 @@ def _run(cli: "bittensor.cli", subtensor: "bittensor.subtensor"): return try: - netuids = ( - subtensor.get_all_subnet_netuids() - if cli.config.is_set("all") - else [netuid] - ) + netuids = subtensor.get_all_subnet_netuids() if cli.config.all else [netuid] hotkey_stake = GetChildrenCommand.get_parent_stake_info( console, subtensor, hotkey ) @@ -1236,7 +1255,7 @@ def add_args(parser: argparse.ArgumentParser): parser = parser.add_parser( "get_children", help="""Get child hotkeys on subnet.""" ) - parser.add_argument("--netuid", dest="netuid", type=int, required=False) + parser.add_argument("--netuid", dest="netuid", type=str, required=False) parser.add_argument("--hotkey", dest="hotkey", type=str, required=False) parser.add_argument( "--all", @@ -1294,7 +1313,7 @@ def render_table( # Add columns to the table with specific styles table.add_column("Index", style="bold yellow", no_wrap=True, justify="center") - table.add_column("ChildHotkey", style="bold green") + table.add_column("Child Hotkey", style="bold green") table.add_column("Proportion", style="bold cyan", no_wrap=True, justify="right") table.add_column( "Childkey Take", style="bold blue", no_wrap=True, justify="right" diff --git a/tests/e2e_tests/subcommands/stake/test_childkeys.py b/tests/e2e_tests/subcommands/stake/test_childkeys.py index 080d01263..a8f6518fc 100644 --- a/tests/e2e_tests/subcommands/stake/test_childkeys.py +++ b/tests/e2e_tests/subcommands/stake/test_childkeys.py @@ -54,7 +54,7 @@ async def test_set_revoke_children_multiple(local_chain, capsys): assert local_chain.query("SubtensorModule", "NetworksAdded", [1]).serialize() for exec_command in [alice_exec_command, bob_exec_command, eve_exec_command]: - exec_command(RegisterCommand, ["s", "register", "--netuid", "1"]) + exec_command(RegisterCommand, ["s", "register", "--netuid", "4"]) alice_exec_command(StakeCommand, ["stake", "add", "--amount", "100000"]) @@ -75,8 +75,8 @@ async def wait(): await wait() children_with_proportions = [ - [0.4, bob_keypair.ss58_address], - [0.2, eve_keypair.ss58_address], + [0.2, bob_keypair.ss58_address], + [0.1, eve_keypair.ss58_address], ] # Test 1: Set multiple children @@ -86,7 +86,7 @@ async def wait(): "stake", "set_children", "--netuid", - "1", + "2", "--children", f"{children_with_proportions[0][1]},{children_with_proportions[1][1]}", "--hotkey", From 4379c596fa4a21c27c21cc9a08410be9e4c904d9 Mon Sep 17 00:00:00 2001 From: opendansor Date: Wed, 28 Aug 2024 17:09:10 -0700 Subject: [PATCH 2/9] CHK Test --- tests/e2e_tests/subcommands/stake/test_childkeys.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/e2e_tests/subcommands/stake/test_childkeys.py b/tests/e2e_tests/subcommands/stake/test_childkeys.py index a8f6518fc..080d01263 100644 --- a/tests/e2e_tests/subcommands/stake/test_childkeys.py +++ b/tests/e2e_tests/subcommands/stake/test_childkeys.py @@ -54,7 +54,7 @@ async def test_set_revoke_children_multiple(local_chain, capsys): assert local_chain.query("SubtensorModule", "NetworksAdded", [1]).serialize() for exec_command in [alice_exec_command, bob_exec_command, eve_exec_command]: - exec_command(RegisterCommand, ["s", "register", "--netuid", "4"]) + exec_command(RegisterCommand, ["s", "register", "--netuid", "1"]) alice_exec_command(StakeCommand, ["stake", "add", "--amount", "100000"]) @@ -75,8 +75,8 @@ async def wait(): await wait() children_with_proportions = [ - [0.2, bob_keypair.ss58_address], - [0.1, eve_keypair.ss58_address], + [0.4, bob_keypair.ss58_address], + [0.2, eve_keypair.ss58_address], ] # Test 1: Set multiple children @@ -86,7 +86,7 @@ async def wait(): "stake", "set_children", "--netuid", - "2", + "1", "--children", f"{children_with_proportions[0][1]},{children_with_proportions[1][1]}", "--hotkey", From 4b046c3351ce2695445a789bdc7a5d1836ac26a9 Mon Sep 17 00:00:00 2001 From: opendansor Date: Thu, 29 Aug 2024 11:29:07 -0700 Subject: [PATCH 3/9] u16 float limit --- bittensor/commands/stake.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bittensor/commands/stake.py b/bittensor/commands/stake.py index 24868d048..eff415d1a 100644 --- a/bittensor/commands/stake.py +++ b/bittensor/commands/stake.py @@ -58,7 +58,7 @@ def get_netuid( ) return False, -1 netuid = cli.config.netuid - if netuid < 0 or netuid > 2**32 - 1: + if netuid < 0 or netuid > 65535: console.print( "[red]Invalid input. Please enter a valid integer for netuid in subnet range.[/red]" ) From 49fd8081982d20443a2ffc54dcc66bebde390846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=B5?= Date: Sun, 11 Aug 2024 15:33:16 +0000 Subject: [PATCH 4/9] btcli/bt.config: various improvements - implement recursive apply function for (sub^n)argparser - use recursive apply to eliminate repeated code - move adding standard bt.config args to separate function - *use recursive apply to add standard bt.config args to all parsers - *make strict=True the default - eliminate redundant COMMANDS.item.name field - remove obsolete COMMANDS looping code - remove redundant dest= argument - fix some comments The changes marked * implement a functional improvement, the rest is for improved maintainability. --- bittensor/cli.py | 33 ++++------ bittensor/config.py | 142 ++++++++++++++++++++++---------------------- 2 files changed, 81 insertions(+), 94 deletions(-) diff --git a/bittensor/cli.py b/bittensor/cli.py index cc87c122e..67e78ec5e 100644 --- a/bittensor/cli.py +++ b/bittensor/cli.py @@ -106,7 +106,6 @@ } COMMANDS = { "subnets": { - "name": "subnets", "aliases": ["s", "subnet"], "help": "Commands for managing and viewing subnetworks.", "commands": { @@ -120,7 +119,6 @@ }, }, "root": { - "name": "root", "aliases": ["r", "roots"], "help": "Commands for managing and viewing the root network.", "commands": { @@ -142,7 +140,6 @@ }, }, "wallet": { - "name": "wallet", "aliases": ["w", "wallets"], "help": "Commands for managing and viewing wallets.", "commands": { @@ -167,7 +164,6 @@ }, }, "stake": { - "name": "stake", "aliases": ["st", "stakes"], "help": "Commands for staking and removing stake and setting child hotkey accounts.", "commands": { @@ -182,7 +178,6 @@ }, }, "weights": { - "name": "weights", "aliases": ["wt", "weight"], "help": "Commands for managing weight for subnets.", "commands": { @@ -191,7 +186,6 @@ }, }, "sudo": { - "name": "sudo", "aliases": ["su", "sudos"], "help": "Commands for subnet management", "commands": { @@ -201,7 +195,6 @@ }, }, "legacy": { - "name": "legacy", "aliases": ["l"], "help": "Miscellaneous commands.", "commands": { @@ -210,7 +203,6 @@ }, }, "info": { - "name": "info", "aliases": ["i"], "help": "Instructions for enabling autocompletion for the CLI.", "commands": { @@ -304,21 +296,18 @@ def __create_parser__() -> "argparse.ArgumentParser": # Add arguments for each sub-command. cmd_parsers = parser.add_subparsers(dest="command") # Add argument parsers for all available commands. - for command in COMMANDS.values(): - if isinstance(command, dict): - subcmd_parser = cmd_parsers.add_parser( - name=command["name"], - aliases=command["aliases"], - help=command["help"], - ) - subparser = subcmd_parser.add_subparsers( - help=command["help"], dest="subcommand", required=True - ) + for name, command in COMMANDS.items(): + subcmd_parser = cmd_parsers.add_parser( + name=name, + aliases=command["aliases"], + help=command["help"], + ) + subparser = subcmd_parser.add_subparsers( + help=command["help"], dest="subcommand", required=True + ) - for subcommand in command["commands"].values(): - subcommand.add_args(subparser) - else: - command.add_args(cmd_parsers) + for subcommand in command["commands"].values(): + subcommand.add_args(subparser) return parser diff --git a/bittensor/config.py b/bittensor/config.py index 59ad4451b..d6c7c8e79 100644 --- a/bittensor/config.py +++ b/bittensor/config.py @@ -64,7 +64,7 @@ def __init__( self, parser: argparse.ArgumentParser = None, args: Optional[List[str]] = None, - strict: bool = False, + strict: bool = True, default: Optional[Any] = None, ) -> None: super().__init__(default) @@ -74,50 +74,7 @@ def __init__( if parser == None: return None - # Optionally add config specific arguments - try: - parser.add_argument( - "--config", - type=str, - help="If set, defaults are overridden by passed file.", - ) - except: - # this can fail if --config has already been added. - pass - - try: - parser.add_argument( - "--strict", - action="store_true", - help="""If flagged, config will check that only exact arguments have been set.""", - default=False, - ) - except: - # this can fail if --strict has already been added. - pass - - try: - parser.add_argument( - "--no_version_checking", - action="store_true", - help="Set ``true`` to stop cli version checking.", - default=False, - ) - except: - # this can fail if --no_version_checking has already been added. - pass - - try: - parser.add_argument( - "--no_prompt", - dest="no_prompt", - action="store_true", - help="Set ``true`` to stop cli from prompting the user.", - default=False, - ) - except: - # this can fail if --no_version_checking has already been added. - pass + config.apply_to_parser_recursive(parser, config.add_standard_args) # Get args from argv if not passed in. if args == None: @@ -182,37 +139,21 @@ def __init__( default_param_args = [_config.get("command"), _config.get("subcommand")] ## Get all args by name - default_params = parser.parse_args(args=default_param_args) + parser_no_required = copy.deepcopy(parser) + for i in range(len(parser_no_required._actions)): + parser_no_required._actions[i].required = False + default_params = parser_no_required.parse_args(args=default_param_args) all_default_args = default_params.__dict__.keys() | [] ## Make a dict with keys as args and values as argparse.SUPPRESS defaults_as_suppress = {key: argparse.SUPPRESS for key in all_default_args} - ## Set the defaults to argparse.SUPPRESS, should remove them from the namespace - parser_no_defaults.set_defaults(**defaults_as_suppress) - parser_no_defaults._defaults.clear() # Needed for quirk of argparse - - ### Check for subparsers and do the same - if parser_no_defaults._subparsers != None: - for action in parser_no_defaults._subparsers._actions: - # Should only be the "command" subparser action - if isinstance(action, argparse._SubParsersAction): - # Set the defaults to argparse.SUPPRESS, should remove them from the namespace - # Each choice is the keyword for a command, we need to set the defaults for each of these - ## Note: we also need to clear the _defaults dict for each, this is a quirk of argparse - cmd_parser: argparse.ArgumentParser - for cmd_parser in action.choices.values(): - # If this choice is also a subparser, set defaults recursively - if cmd_parser._subparsers: - for action in cmd_parser._subparsers._actions: - # Should only be the "command" subparser action - if isinstance(action, argparse._SubParsersAction): - cmd_parser: argparse.ArgumentParser - for cmd_parser in action.choices.values(): - cmd_parser.set_defaults(**defaults_as_suppress) - cmd_parser._defaults.clear() # Needed for quirk of argparse - else: - cmd_parser.set_defaults(**defaults_as_suppress) - cmd_parser._defaults.clear() # Needed for quirk of argparse + + def l_set_defaults(l_parser): + ## Set the defaults to argparse.SUPPRESS, should remove them from the namespace + l_parser.set_defaults(**defaults_as_suppress) + l_parser._defaults.clear() # Needed for quirk of argparse + + config.apply_to_parser_recursive(parser_no_defaults, l_set_defaults) ## Reparse the args, but this time with the defaults as argparse.SUPPRESS params_no_defaults = config.__parse_args__( @@ -231,6 +172,63 @@ def __init__( ] } + @staticmethod + def apply_to_parser_recursive(parser, callback, depth=0): + callback(parser) + if not parser._subparsers: + return + for action in parser._subparsers._actions: + if not isinstance(action, argparse._SubParsersAction): + continue + for cmd_parser in action.choices.values(): + config.apply_to_parser_recursive(cmd_parser, callback, depth=depth + 1) + + @staticmethod + def add_standard_args(parser): + # Optionally add config specific arguments + try: + parser.add_argument( + "--config", + type=str, + help="If set, defaults are overridden by passed file.", + ) + except: + # this can fail if --config has already been added. + pass + + try: + parser.add_argument( + "--strict", + action="store_true", + help="If flagged, config will check that only exact arguments have been set.", + default=False, + ) + except: + # this can fail if --strict has already been added. + pass + + try: + parser.add_argument( + "--no_version_checking", + action="store_true", + help="Set ``true`` to stop cli version checking.", + default=False, + ) + except: + # this can fail if --no_version_checking has already been added. + pass + + try: + parser.add_argument( + "--no_prompt", + action="store_true", + help="Set ``true`` to stop cli from prompting the user.", + default=False, + ) + except Exception as e: + # this can fail if --no_prompt has already been added. + pass + @staticmethod def __split_params__(params: argparse.Namespace, _config: "config"): # Splits params on dot syntax i.e neuron.axon_port and adds to _config From dbb444a015284e6852de81488a21cb1ddf86a2d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=B5?= Date: Wed, 14 Aug 2024 07:04:32 +0000 Subject: [PATCH 5/9] first round of style feedback --- bittensor/config.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/bittensor/config.py b/bittensor/config.py index d6c7c8e79..0d15c0cf0 100644 --- a/bittensor/config.py +++ b/bittensor/config.py @@ -26,7 +26,7 @@ import copy from copy import deepcopy from munch import DefaultMunch -from typing import List, Optional, Dict, Any, TypeVar, Type +from typing import List, Optional, Dict, Any, TypeVar, Type, Callable import argparse @@ -74,7 +74,7 @@ def __init__( if parser == None: return None - config.apply_to_parser_recursive(parser, config.add_standard_args) + config.apply_to_parser_recursive(parser, config.add_args) # Get args from argv if not passed in. if args == None: @@ -148,12 +148,12 @@ def __init__( ## Make a dict with keys as args and values as argparse.SUPPRESS defaults_as_suppress = {key: argparse.SUPPRESS for key in all_default_args} - def l_set_defaults(l_parser): + def local_set_defaults(local_parser:argparse.ArgumentParser): ## Set the defaults to argparse.SUPPRESS, should remove them from the namespace - l_parser.set_defaults(**defaults_as_suppress) - l_parser._defaults.clear() # Needed for quirk of argparse + local_parser.set_defaults(**defaults_as_suppress) + local_parser._defaults.clear() # Needed for quirk of argparse - config.apply_to_parser_recursive(parser_no_defaults, l_set_defaults) + config.apply_to_parser_recursive(parser_no_defaults, local_set_defaults) ## Reparse the args, but this time with the defaults as argparse.SUPPRESS params_no_defaults = config.__parse_args__( @@ -173,7 +173,10 @@ def l_set_defaults(l_parser): } @staticmethod - def apply_to_parser_recursive(parser, callback, depth=0): + def apply_to_parser_recursive(parser:argparse.ArgumentParser, callback:Callable[[argparse.ArgumentParser],None], depth:int=0): + """ + Recursively apply callback() to parser and its subparsers. + """ callback(parser) if not parser._subparsers: return @@ -184,7 +187,10 @@ def apply_to_parser_recursive(parser, callback, depth=0): config.apply_to_parser_recursive(cmd_parser, callback, depth=depth + 1) @staticmethod - def add_standard_args(parser): + def add_args(parser:argparse.ArgumentParser): + """ + Add standard arguments to argument parser. + """ # Optionally add config specific arguments try: parser.add_argument( From 907100da776c0d273c95edc5f91b6a3ac240cfb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=B5?= Date: Wed, 14 Aug 2024 07:06:37 +0000 Subject: [PATCH 6/9] tests/integration_tests/test_cli_no_network.py: drop unrecognized argument 'hyperparameters' --- tests/integration_tests/test_cli_no_network.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration_tests/test_cli_no_network.py b/tests/integration_tests/test_cli_no_network.py index e3a3d6a49..bcf8268d8 100644 --- a/tests/integration_tests/test_cli_no_network.py +++ b/tests/integration_tests/test_cli_no_network.py @@ -1384,7 +1384,6 @@ def _test_value_parsing(param: str, value: str): args=[ "sudo", "set", - "hyperparameters", "--netuid", "1", "--param", From 4460e08d213ce69f59f194354cc4a6c37b903f2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=B5?= Date: Wed, 14 Aug 2024 07:12:03 +0000 Subject: [PATCH 7/9] tests/integration_tests/test_cli_no_network.py: fix unrecognized argument '--proposal_hash' --- tests/integration_tests/test_cli_no_network.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/test_cli_no_network.py b/tests/integration_tests/test_cli_no_network.py index bcf8268d8..3eca077ae 100644 --- a/tests/integration_tests/test_cli_no_network.py +++ b/tests/integration_tests/test_cli_no_network.py @@ -1324,7 +1324,7 @@ def test_vote_command_prompt_proposal_hash(self, _): cli = bittensor.cli( args=base_args + [ - "--proposal_hash", + "--proposal", mock_proposal_hash, ] ) From 8c7d191b88186e3c6e210bac7763592784af6a9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=B5?= Date: Wed, 14 Aug 2024 19:25:58 +0000 Subject: [PATCH 8/9] ruff bittensor/config.py --- bittensor/config.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/bittensor/config.py b/bittensor/config.py index 0d15c0cf0..95e49cd2d 100644 --- a/bittensor/config.py +++ b/bittensor/config.py @@ -148,7 +148,7 @@ def __init__( ## Make a dict with keys as args and values as argparse.SUPPRESS defaults_as_suppress = {key: argparse.SUPPRESS for key in all_default_args} - def local_set_defaults(local_parser:argparse.ArgumentParser): + def local_set_defaults(local_parser: argparse.ArgumentParser): ## Set the defaults to argparse.SUPPRESS, should remove them from the namespace local_parser.set_defaults(**defaults_as_suppress) local_parser._defaults.clear() # Needed for quirk of argparse @@ -173,7 +173,11 @@ def local_set_defaults(local_parser:argparse.ArgumentParser): } @staticmethod - def apply_to_parser_recursive(parser:argparse.ArgumentParser, callback:Callable[[argparse.ArgumentParser],None], depth:int=0): + def apply_to_parser_recursive( + parser: argparse.ArgumentParser, + callback: Callable[[argparse.ArgumentParser], None], + depth: int = 0, + ): """ Recursively apply callback() to parser and its subparsers. """ @@ -187,7 +191,7 @@ def apply_to_parser_recursive(parser:argparse.ArgumentParser, callback:Callable[ config.apply_to_parser_recursive(cmd_parser, callback, depth=depth + 1) @staticmethod - def add_args(parser:argparse.ArgumentParser): + def add_args(parser: argparse.ArgumentParser): """ Add standard arguments to argument parser. """ From d7cb599e3cafe11475b56f3c87ebd6e3b03188ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=B5?= Date: Wed, 14 Aug 2024 19:26:43 +0000 Subject: [PATCH 9/9] tests/integration_tests/test_cli.py remove superfluous --wallet.name from test By using patch() to mock bittensor.wallet, the mechanics of bittensor.wallet are skipped, including adding the args for parser. They have no effect and are not recognized, leading to an error. --- tests/integration_tests/test_cli.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/integration_tests/test_cli.py b/tests/integration_tests/test_cli.py index 6fe1acf3b..903f7cee9 100644 --- a/tests/integration_tests/test_cli.py +++ b/tests/integration_tests/test_cli.py @@ -2506,8 +2506,6 @@ def test_delegate(self, _): "delegate", "--subtensor.network", "mock", # Mock network - "--wallet.name", - "mock", "--delegate_ss58key", delegate_wallet.hotkey.ss58_address, "--amount",