Add support for Optional[Tuple[...]] (#115) #116
Conversation
lib/defopt.py
Outdated
| @@ -699,7 +703,7 @@ def _get_type_from_doc(name, globalns): | |||
| # Support for sphinx-specific "list[type]", "tuple[type]" syntax; only | |||
| # needed for Py<3.9. | |||
| # (This intentionally won't catch `List` or `typing.List`.) | |||
| match = re.match(r'(list|tuple)\[([\w\.]+)\]', name) | |||
| match = re.match(r'(list|tuple)\[([\w\.,]+)\]', name) | |||
There was a problem hiding this comment.
the comma is specifically for the case where you have a multiple-item tuple + union operator typed in a docstring rather than a type hint
neelmraman
force-pushed
the
optional_tuple
branch
from
c62a6be
to
e456472
Compare
lib/defopt.py
Outdated
| raise ValueError( | ||
| "Optional tuples of length greater than one are not " | ||
| "supported with use of a NoneType parser due to ambiguity." | ||
| ) |
There was a problem hiding this comment.
I guess we should also disable support for 1-element tuples for now, per #115 (comment)?
|
Mostly looks good (in light of #115 (comment)), just one comment. |
neelmraman
force-pushed
the
optional_tuple
branch
from
e456472
to
a47f6ff
Compare
|
I allowed myself to push various small tweaks directly to your branch; will squash-merged once tests pass and if you agree with the changes I made. |
test_defopt.py
Outdated
| def _parse_none(i): | ||
| if i.lower() == 'none': | ||
| return None | ||
| else: | ||
| raise ValueError('{} is not a valid None string'.format(i)) | ||
| with self.assertRaises(ValueError): | ||
| self.assertEqual( |
There was a problem hiding this comment.
Ah yeah whoops the self.assertEqual was a typo from copy/pasting. Thanks for catching.
|
Yup the changes look good. Most of the formatting differences seem to be from me running my changes through black, but happy to defer to whatever autoformatting tool/style convention you're using instead. |
Closes #115