perf: support for custom action in config parser #361
Conversation
| @@ -45,10 +47,39 @@ def arg( | |||
| "short": short, | |||
| "countable": countable, | |||
| "global_default_str": global_default_str, | |||
| "action": action, | |||
There was a problem hiding this comment.
would it be better to use a different name, e.g., custom_action, to avoid potential name clashes? it might be fine as the name choices is also used here, but let me know if you have any concerns.
There was a problem hiding this comment.
not sure I follow, name clash with what?
I'm ok with custom_action, but don't love the "action" nomenclature. If this is about parsing, what about parser?
There was a problem hiding this comment.
parser is even more overloaded, ignore that. Either action or custom_action works IMO, I wouldn't worry about name clashes at that level (I thought this was a concern about shadowing)
There was a problem hiding this comment.
okay, i will stick with action.
| }, | ||
| ) | ||
|
|
||
|
|
||
| class ParseCSV(argparse.Action): |
There was a problem hiding this comment.
what do we get by subclassing argparse.Action?
There was a problem hiding this comment.
ah I see we pass it to the generated argparse parser. Pretty cool!
| @@ -45,10 +47,39 @@ def arg( | |||
| "short": short, | |||
| "countable": countable, | |||
| "global_default_str": global_default_str, | |||
| "action": action, | |||
There was a problem hiding this comment.
parser is even more overloaded, ignore that. Either action or custom_action works IMO, I wouldn't worry about name clashes at that level (I thought this was a concern about shadowing)
| }, | ||
| ) | ||
|
|
||
|
|
||
| class ParseCSV(argparse.Action): |
There was a problem hiding this comment.
ah I see we pass it to the generated argparse parser. Pretty cool!
add support for custom action in halmos config. this allows structural option values (e.g., --array-length) to be parsed at the time of config creation.
note: since the config object is frozen, it is nontrivial to extend or update the config with parsing raw string values after config has been created. also, since config objects are re-generated to override further configurations, it's more convenient to parse raw string through argparser.