Type inference #1
Conversation
| elif var.lower() == 'false': | ||
| return False | ||
| else: | ||
| raise ArgumentTypeError() |
There was a problem hiding this comment.
So I think one thing is that we'd want to allow e.g. int : if a user has False or None in the config, they might want to pass in another param.
Perhaps we could express these as just a "priority" list of types?
src/configmypy/type_inference.py
Outdated
| return float(var) | ||
| elif var.isnumeric(): | ||
| return int(var) | ||
| else: |
There was a problem hiding this comment.
Maybe we can default to str and support None?
src/configmypy/argparse_config.py
Outdated
| @@ -62,7 +62,13 @@ def read_conf(self, config=None, **additional_config): | |||
| parser = argparse.ArgumentParser(description='Read the config from the commandline.') | |||
| for key, value in iter_nested_dict_flat(config, return_intermediate_keys=self.overwrite_nested_config): | |||
| if self.infer_types and value is not None: | |||
| parser.add_argument(f'--{key}', type=type(value), default=value) | |||
| # use type inferencing callable if available, otherwise strictly infer type | |||
| if type(value) == int or type(value) == float: | |||
There was a problem hiding this comment.
Do you think we should tie the parser to ArgParse Specifically or should we make it generic?
e.g.
class TypeInferencer:
def __init__(self, original_type, strict=False):
...
def infer(self, var):
if self.strict:
return original_type(var)
else:
# infer type smartlyThere was a problem hiding this comment.
this may cause issues, since argparse will infer bool('False') => True
There was a problem hiding this comment.
Where should the inference go if not in between the parser and the config?
There was a problem hiding this comment.
Right that's what I mean -- we need to have an option to use our own type inference rather than relying on argparse which can be too rigid. It can be just in a separate file (e.g. type_inference.py and just used in the argparse config)
There was a problem hiding this comment.
It looks great, thanks @dhpitt -- just realized we have a slight inconsistency with the arg name.
|
Thanks @dhpitt, merging! |
Argparse directly takes custom Callable functions as a type.
This PR: