-
Notifications
You must be signed in to change notification settings - Fork 40
Add ability to override models_dir #21
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
Conversation
WalkthroughThe changes introduce a new command-line option for specifying a directory containing RVC models. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- README.md (1 hunks)
- rvc_python/main.py (2 hunks)
Additional comments not posted (2)
rvc_python/__main__.py (1)
43-43: LGTM!The
RVCInferenceclass instantiation correctly includes themodels_dirparameter.The code changes are approved.
README.md (1)
247-247: LGTM!The documentation correctly includes the new
--models_dirargument, providing users with the necessary information to utilize the new feature.The code changes are approved.
| # Common arguments for both CLI and API | ||
| for subparser in [cli_parser, api_parser]: | ||
| subparser.add_argument("-mp", "--model", type=str, required=True, help="Path to model file") | ||
| subparser.add_argument("-md", "--models_dir", type=str, default="rvc_models", help="Directory to store models") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add validation for models_dir argument.
The models_dir argument should be validated to ensure the directory exists or can be created. This will prevent runtime errors if the directory is invalid.
Apply this diff to add validation for the models_dir argument:
args = parser.parse_args()
+ # Validate models_dir
+ if not os.path.exists(args.models_dir):
+ try:
+ os.makedirs(args.models_dir)
+ except OSError as e:
+ print(f"Error: Unable to create models directory '{args.models_dir}'. {e}")
+ sys.exit(1)
# Initialize RVCInferenceCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| subparser.add_argument("-md", "--models_dir", type=str, default="rvc_models", help="Directory to store models") | |
| args = parser.parse_args() | |
| # Validate models_dir | |
| if not os.path.exists(args.models_dir): | |
| try: | |
| os.makedirs(args.models_dir) | |
| except OSError as e: | |
| print(f"Error: Unable to create models directory '{args.models_dir}'. {e}") | |
| sys.exit(1) | |
| # Initialize RVCInference |
Adds an argparser variable for models_dir. The default is
rvc_modelsas before, but is overridable now.Summary by CodeRabbit
--models_dirfor specifying the directory of RVC models, enhancing configurability and usability.rvc_models, allowing for more flexible model management.