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

bittensor/config.py: improvements #2222

Closed

Conversation

mvds00
Copy link

@mvds00 mvds00 commented Aug 11, 2024

Bug

  • You can pass any argument to code that uses bt.config() with default kwargs and it will not complain. This leads to confusion.
  • bt.config() does some special magic that kills the required=True feature of argparse; it will complain that the argument is not set, even when it is set. This leads to devs implementing workarounds such as setting semi-random default values and/or additional post-argparse checks that could/should already be done in argparse.

Description of the Change

  • set strict=True by default, so issues are easier to catch
  • fix the inability to handle required=True

Alternate Designs

Rework bt.config, as it is not clear what it intends to add to argparse, that already supports tons of features.

Possible Drawbacks

People may experience code not starting anymore because they supply unknown arguments that now lead to errors due to strict checking. This is intended. Having non-functional arguments is not useful and confusing.

Verification Process

I tested this on some bittensor code I'm writing right now, by adding a required=True argument and observing that the argument is now required and that the code doesn't bail out on a supposedly missing argument. I tested to see that I now cannot add --whatevercrapinsertedhere without argparse complaining about an unknown arg.

Release Notes

N/A

- by default, strict=True so issues are easier to catch
- fix the inability to handle required=True
@mvds00 mvds00 closed this Aug 11, 2024
@mvds00 mvds00 deleted the features/mvds00/improve-bt-config branch August 11, 2024 15:11
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.

1 participant