Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic API: Support positional-only arguments #4660

Closed
pekkaklarck opened this issue Feb 15, 2023 · 1 comment · Fixed by #4701
Closed

Dynamic API: Support positional-only arguments #4660

pekkaklarck opened this issue Feb 15, 2023 · 1 comment · Fixed by #4701
Labels
acknowledge To be acknowledged in release notes beta 1 effort: small enhancement pr Pull request exists priority: medium
Milestone

Comments

@pekkaklarck
Copy link
Member

RF 4.0 added support for Python's positional-only arguments (#3695). It isn't that important feature, but sometimes it comes handy. For consistency reasons also dynamic libraries should support it.

Implementing this enhancement shouldn't be too complicated. Our ArgumentSpec already supports positional-arguments and that's what is used during execution. The only needed change ought to be enhancing the code that parses argument information returned by dynamic libraries. Well, even before that we needed to agree on the syntax but ['posonly', '/', 'normal'] that matches the syntax used by Python is a pretty obvious candidate. It's also consistent how the dynamic API supports named-only arguments like ['normal', '*', 'namedonly'].

@pekkaklarck
Copy link
Member Author

If someone is interested to contribute, the parsers for different arguments are here:
https://github.com/robotframework/robotframework/blob/master/src/robot/running/arguments/argumentparser.py

@pekkaklarck pekkaklarck added the pr Pull request exists label Mar 27, 2023
@pekkaklarck pekkaklarck added this to the v6.1 milestone Mar 27, 2023
@pekkaklarck pekkaklarck added the acknowledge To be acknowledged in release notes label Apr 3, 2023
pekkaklarck added a commit that referenced this issue Apr 3, 2023
Make, for example, `['*', 'kwo', '/', '???']` and `['**kws', '/']`
explicitly invalid. Part of #4660.
pekkaklarck added a commit that referenced this issue Apr 3, 2023
1. Remove new "Positional only argument syntax with dynamic libraries"
   section added as part of #4660. It mostly explains how positional
   arguments work in general, but that's alrady explained
   elsewhere. We can consider adding adding it back so that it
   explains how they work with the dynamic API, but I'm not sure it's
   needed. We don't have such a section with `*varargs` either and
   believe the remaining documentation and examples explain this
   topic well enough.

2. Enhance formatting of examples related to argument specs returned
   by `get_keyword_arguments`.

3. Make names of tests in examples more clear. Most importantly, avoid
  "Positional only" term when not actually talking about
  positional-only arguments.
pekkaklarck added a commit that referenced this issue Apr 4, 2023
Make, for example, `['*', 'kwo', '/', '???']` and `['**kws', '/']`
explicitly invalid. Part of #4660.
pekkaklarck added a commit that referenced this issue Apr 4, 2023
1. Remove new "Positional only argument syntax with dynamic libraries"
   section added as part of #4660. It mostly explains how positional
   arguments work in general, but that's alrady explained
   elsewhere. We can consider adding adding it back so that it
   explains how they work with the dynamic API, but I'm not sure it's
   needed. We don't have such a section with `*varargs` either and
   believe the remaining documentation and examples explain this
   topic well enough.

2. Enhance formatting of examples related to argument specs returned
   by `get_keyword_arguments`.

3. Make names of tests in examples more clear. Most importantly, avoid
  "Positional only" term when not actually talking about
  positional-only arguments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledge To be acknowledged in release notes beta 1 effort: small enhancement pr Pull request exists priority: medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant