Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactoring concept: generate typer commands from config #258

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

LeaveMyYard
Copy link
Contributor

@LeaveMyYard LeaveMyYard commented Apr 4, 2024

Related to #219

This is a proposal to move typer command generation to a separate file, restructure components in functions so it is easier to understand what is going on, and to use Pydantic's field extra params to generate typer commands from Config.

Pros:

  • Now to add a new flag it is only required to add it to config
  • Was able to refactor the way a strategy is created, so now it is the same mechanism with strategy settings, so it is easier to understand
  • Code is easier to read for newcomers (it is easier to understand how commands are generated)
  • Data validation and setup of how it is described in --help is now in the same place, and that is much more convenient

Cons:

  • We still use inspect in runtime

NOTE: This is not yet finished, but rather an idea on the change

@LeaveMyYard LeaveMyYard requested a review from aantn April 4, 2024 16:12
@LeaveMyYard LeaveMyYard self-assigned this Apr 4, 2024
@LeaveMyYard
Copy link
Contributor Author

@aantn WDYT?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant