keyword-only arguments as required Option #270
Conversation
Codecov Report
@@ Coverage Diff @@
## master #270 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 242 242
Lines 4508 4509 +1
=========================================
+ Hits 4508 4509 +1
Continue to review full report at Codecov.
|
| name: str, | ||
| default: Any = inspect.Parameter.empty, | ||
| annotation: Any = inspect.Parameter.empty, | ||
| kind: inspect._ParameterKind = inspect.Parameter.POSITIONAL_OR_KEYWORD, |
There was a problem hiding this comment.
accessing inspect._ParameterKind is ugly a bit, but the type name is _ParameterKind
https://github.com/python/typeshed/blob/a690a14c820238ca7a64cd502343d6b5a649e731/stdlib/inspect.pyi#L125-L134
| elif param.default == Required or param.default == param.empty: | ||
| required = True | ||
| parameter_info = ArgumentInfo() | ||
| if param.kind == inspect.Parameter.KEYWORD_ONLY: |
There was a problem hiding this comment.
Ideally, there would be a tutorial file in docs_src demonstrating the behavior. And then a test for that file in tests/test_tutorial/. Then reference the file in the markdown documentation with a brief explanation.
Sometimes there are tricky edge cases that don't fit into tutorials. In that case, you can add some tests in the top level of the tests/ directory.
|
@podhmo do you need some help on this PR? I really feel like this feature would be game breaker! |
|
@tiangolo What is missing in this PR so that it can be accepted? How could I help making it complete? |
|
Oh, sorry for late response, the missing part is tests. |
ryangalamb
left a comment
ryangalamb
left a comment
There was a problem hiding this comment.
As pointed out earlier, tests are the main thing blocking this right now.
FWIW, this is a breaking change (existing CLIs that use keyword-only arguments will change behavior), but I think this is useful enough to justify a breaking change.
Let me know if you have any more questions about how/where to add tests.
| elif param.default == Required or param.default == param.empty: | ||
| required = True | ||
| parameter_info = ArgumentInfo() | ||
| if param.kind == inspect.Parameter.KEYWORD_ONLY: |
There was a problem hiding this comment.
Ideally, there would be a tutorial file in docs_src demonstrating the behavior. And then a test for that file in tests/test_tutorial/. Then reference the file in the markdown documentation with a brief explanation.
Sometimes there are tricky edge cases that don't fit into tutorials. In that case, you can add some tests in the top level of the tests/ directory.
|
Thank you for the comments, but sorry, I've lost the motivation. So, feel free to reopen or reuse the code in new pull request. |
|
Sounds good, thanks for responding and sharing your code/time! |
3 participants
supports #269