Fix incorrect behavior in argv.StoreInModule #389
Conversation
Also document it now that it works :) Previously, this would always try to set attributes in the openhtf module itself, not openhtf.util.logs or whichever module was correct.
| @@ -19,5 +35,7 @@ def __init__(self, *args, **kwargs): | |||
| def __call__(self, parser, namespace, values, option_string=None): | |||
| if hasattr(self, '_proxy'): | |||
| values = self._proxy(parser, namespace, values) | |||
| setattr(__import__(self._tgt_mod), self._tgt_attr, values) | |||
| base, mod = self._tgt_mod.rsplit('.', 1) | |||
There was a problem hiding this comment.
This unpacking will break if the target module is already a base module without any dots, but arguably it doesn't make sense to use StoreInModule in such a case.
There was a problem hiding this comment.
hm, yeah it's nonsensical to have no dots, but something one might accidentally do if you forget to do the %s/name business, maybe throw in a quick format check on the inputs of add_argument() to fail-fast rather than raise an obscure error here?
|
LGTM, we'll get a merge conflict on station_api from the fix I threw in here, but farz's version is better, I think, mine was just a quick make-it-work fix: |
|
I'm gonna merge this now so that the onus will be on me to resolve that conflict when rebasing station_api in prep for the PR to merge it into master. Created #390 to get a better error message if someone sets an unsuitable target. |
Also document it now that it works :)
Previously, this would always try to set attributes in the openhtf module itself, not openhtf.util.logs or whichever module was correct.