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

Switch from argparse to click for argument parsing #1184

Closed
wants to merge 6 commits into from
Closed

Switch from argparse to click for argument parsing #1184

wants to merge 6 commits into from

Conversation

Swiftyos
Copy link
Contributor

Background

This PR changes the argument parsing library from argparse to click. The primary reason for this switch is to leverage the benefits provided by click, while maintaining backward compatibility with the existing implementation. Click is a powerful and flexible library for creating command-line interfaces, offering several advantages over argparse, such as a more intuitive and consistent API, better error messages, automatic generation of help messages, and advanced features like command chaining and nested commands.

Changes

  • Replace argparse with click for argument parsing.
  • Update the main function to use click decorators and options.
  • Renamed the parse_arguments to create_config
  • Ensure that existing functionality is preserved with minimal changes to the codebase.
  • Keep the door open for future enhancements using click's built-in features.

Benefits of Click

  • More intuitive and consistent API.
  • Better error messages and automatic generation of help messages.
  • Advanced features like command chaining and nested commands.
  • Actively maintained and widely used library, with extensive documentation and community support.

Documentation

The code changes include in-code comments to describe the new click decorators and options. The external documentation will be updated to reflect the change from argparse to click.

Test Plan

  1. Manually test the script with various command-line options to ensure that the existing functionality is preserved.
  2. Run existing test cases to ensure that the change does not introduce any regressions.

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts.
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively.
  • I have not snuck in any "extra" small tweaks changes

@BillSchumacher
Copy link
Contributor

<3 click

nponeccop
nponeccop previously approved these changes Apr 14, 2023
@nponeccop
Copy link
Contributor

@0xArty There are conflicts now

@Swiftyos
Copy link
Contributor Author

@nponeccop

Merge conflicts fixed 👍

nponeccop
nponeccop previously approved these changes Apr 14, 2023
@richbeales
Copy link
Contributor

please build in the changes that have just been merged by #1096

@Swiftyos
Copy link
Contributor Author

@richbeales done

@nponeccop nponeccop mentioned this pull request Apr 14, 2023
1 task
@nponeccop
Copy link
Contributor

@0xArty There are conflicts now

@Swiftyos Swiftyos closed this Apr 15, 2023
@Swiftyos
Copy link
Contributor Author

Closed too many merge conflicts.

Will coordinate this change with the team

@Swiftyos Swiftyos mentioned this pull request Apr 18, 2023
5 tasks
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.

4 participants