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.

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

Child Hotkey Refactor Update #48

Merged
merged 8 commits into from
Sep 4, 2024
Merged

Child Hotkey Refactor Update #48

merged 8 commits into from
Sep 4, 2024

Conversation

opendansor
Copy link
Contributor

Refactor indentation for readability

Adjusted indentation in stake.py to maintain consistent code formatting. This change addresses alignment issues in functions, argument lists, and block structures, enhancing the readability and maintainability of the code.

Refactor indentation for readability

Adjusted indentation in `stake.py` to maintain consistent code formatting. This change addresses alignment issues in functions, argument lists, and block structures, enhancing the readability and maintainability of the code.
@opendansor opendansor self-assigned this Sep 3, 2024
@opendansor opendansor marked this pull request as ready for review September 3, 2024 22:36
@opendansor opendansor closed this Sep 3, 2024
@opendansor opendansor reopened this Sep 3, 2024
Corrected indentation across multiple functions in stake.py to maintain code consistency and readability. These changes ensure proper alignment as per Python syntax, improving maintainability and reducing potential bugs.
Corrected inconsistent indentation in multiple sections of stake.py, ensuring proper alignment for readability and code functionality. This change addresses formatting glitches without altering the core logic.
bittensor_cli/cli.py Outdated Show resolved Hide resolved
Add an `all_netuids` parameter to allow fetching children from all subnets. Also, enhance prompts and options for better user experience. Adjust command usage examples accordingly.
Corrected the indentation of the `stake_get_children` method in `bittensor_cli/cli.py` for better readability and code consistency. This change does not modify functionality but improves code structure.
Refactor `netuid` parameter to be an optional string, and add logic to treat "all" as a special case by setting `netuid` to `None`. This improves the flexibility and user input handling in the staking command.
@@ -2804,7 +2804,17 @@ def stake_get_children(
wallet_path: Optional[str] = Options.wallet_path,
network: Optional[str] = Options.network,
chain: Optional[str] = Options.chain,
netuid: int = Options.netuid,
netuid: Optional[str] = typer.Option(
Copy link
Contributor Author

@opendansor opendansor Sep 4, 2024

Choose a reason for hiding this comment

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

This is not Options.netuid because if --all flag is entered we dont want to prompt the user for netuid. (set prompt to false)

Copy link
Contributor

Choose a reason for hiding this comment

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

netuid should be an Optional[int] not str

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See comment below

Copy link
Contributor

@thewhaleking thewhaleking left a comment

Choose a reason for hiding this comment

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

Also please run ruff.

@@ -2804,7 +2804,17 @@ def stake_get_children(
wallet_path: Optional[str] = Options.wallet_path,
network: Optional[str] = Options.network,
chain: Optional[str] = Options.chain,
netuid: int = Options.netuid,
netuid: Optional[str] = typer.Option(
Copy link
Contributor

Choose a reason for hiding this comment

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

netuid should be an Optional[int] not str

netuid = None
elif not netuid:
netuid = Prompt.ask("Netuid")
if netuid and netuid.lower() == "all":
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not work, because netuid should be an int (consistent and also what get_children expects), so does not have the attribute lower.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see comment below.

if all_netuids:
netuid = None
elif not netuid:
netuid = Prompt.ask("Netuid")
Copy link
Contributor

Choose a reason for hiding this comment

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

IntPrompt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For all of the netuid(int) questions here,

If you do not enter netuid in the args, it will prompt you for netuid.
If you enter 'all' it should behave as if you entered the --all flag to get the children from all subnets.

Updated the input prompt for netuid to use IntPrompt, ensuring integer input. Additionally, added a check to prevent specifying both netuid and all_netuids options simultaneously, exiting with an error message if both are given.
Updated the input prompt for netuid to use IntPrompt, ensuring integer input. Additionally, added a check to prevent specifying both netuid and all_netuids options simultaneously, exiting with an error message if both are given.
@thewhaleking thewhaleking merged commit a6bc7b9 into main Sep 4, 2024
2 of 3 checks passed
@thewhaleking thewhaleking deleted the feat/opendansor/chk_all branch September 4, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants