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

added toml processing capability #2369

Merged
merged 30 commits into from
May 17, 2024
Merged

added toml processing capability #2369

merged 30 commits into from
May 17, 2024

Conversation

ovatman
Copy link
Contributor

@ovatman ovatman commented Mar 25, 2024

This PR adds toml config file input capability.

Following changes are provided:

  • a parameter set is added to the command line arguments to input toml file name and profile
  • parse_toml_args function reads all the options available in the toml file provided, taking into account the provided profile name
  • get_option_string_destinationfunction collects the mathing option string according to the provided key in toml file by utilizing the newly added from_option_string method in *Options classes.
  • some methods and classes are moved to the pyk.clipackage to provide better testability
  • unit tests and test harness added for toml input sanity tests
  • required modifications made on code regarding the new functionality and the ones that ar moved to another package.

@ovatman ovatman self-assigned this Apr 25, 2024
@ovatman ovatman added the enhancement New feature or request label Apr 25, 2024
@ovatman ovatman marked this pull request as ready for review April 29, 2024 09:31
@ovatman ovatman requested a review from nwatson22 April 29, 2024 09:31
kevm-pyk/src/kevm_pyk/cli.py Outdated Show resolved Hide resolved
kevm-pyk/src/kevm_pyk/cli.py Show resolved Hide resolved
@ovatman ovatman requested a review from nwatson22 May 10, 2024 14:21

@staticmethod
def get_argument_type() -> dict[str, Callable]:
return {'node': node_id_like, 'node-delta': arg_pair_of(node_id_like, node_id_like)}
Copy link
Member

Choose a reason for hiding this comment

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

I think this is still a type mismatch. KCFGShowOptions.nodes is expected to be a list of node ids, but the TOML reading system expects a single node ID, and will assign KCFGShowOptions.nodes to that int. With the get_argument_type system you added maybe you can just set the argument type to list. This should apply to all the options where the add_argument call for that option has action='append'. That setting makes it so that the typing function specified in add_argument's type parameter no longer matches the type that we want to end up as a field in the options class.

Copy link
Contributor Author

@ovatman ovatman May 15, 2024

Choose a reason for hiding this comment

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

I've modified list_of()function to handle lists provided by toml and apply type transformation to each element of the string. Also modified my test to test for nodesand node_deltasproperly.

@ovatman ovatman merged commit 7b8e3b0 into master May 17, 2024
11 checks passed
@ovatman ovatman deleted the tolga/toml-options branch May 17, 2024 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants