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

Discuss/Consider using all named arguments #1831

Open
emanlove opened this issue May 18, 2023 · 1 comment
Open

Discuss/Consider using all named arguments #1831

emanlove opened this issue May 18, 2023 · 1 comment

Comments

@emanlove
Copy link
Member

Coming from discussions [1] around combining all timeout library arguments and some changes done within the core and used by the browser library there is a thought/proposal to use all named arguments as compared to both named and positional ones within the library import.

[At the moment, this is more of a placeholder then a full description as I myself am still understanding the issue/proposal. But as a way to have an open discussion we want to place this here so we can start to describe, understand and discuss possibilities around this topic.]

[1] https://robotframework.slack.com/archives/C051AB7MFEH/p1681256514114099

@emanlove emanlove added this to the V6.2.0 milestone May 18, 2023
@Snooz82
Copy link
Member

Snooz82 commented Jul 18, 2023

I am not able to read that Slack post. Maybe it was in private?

However:

Why did we (RF Browser) switch from pos_or_named arguments to named_only arguments on import and some keywords?

Browser library has some keyword that have a massive amount of arguments.
Like New Browser or New Context or even more New Persistent Context

Also the library init has 13 arguments or so.

1. This is too much to use it positionally

calling these with positional arguments, the reader of this code would have no clue of which argument is what.

2. These arguments do not have any distinct order.

They are not related to each other.
Like in SL: why is it so, that the action_chain_delay is the last argument? just because it was added last.

see also Click compared to new Click With Options

3. unordered positional arguments are harder to read and understand.

because there is no meaningful oder it is harder to find the correct argument.

4. positional arguments are hard to maintain or extend.

extending an argument interface of positional arguments always mean, to add new ones to the end and never get rid of any of those before. It is impossible to really deprecate an argument, because there is no way user may do the transition from one amount to another amount without breaking it. Exception is, that users use named arguments. then the order is irrelevant.

So named arguments are easy to extend and still keep a good shape/readability.
Specially when the order of the args has anyway no meaning.
See also Take Screenshot where the order of the named ones is irrelevant.
However most of Browser libs getters do have a recognisable order of args.

How did we do the transition?

It was in my opinion a smart and effective solution.

  1. I created a *deprecated_pos_args argument, that would collect all positional arguments. here
  2. I ordered all arguments, which are now named_only, by their name and updated the docs.
  3. I created a dictionary of original order with names and types here
  4. I created some code that would iterate over these deprecated_pos_args assigned values, would interpret them by their position, convert them into the type the should have been and merge them with the named ones.
    see line 791 to 805.
    remark: line 804 params = dict(locals()) should have been the first thing in init before 789...

therefore you can maintain backwards compatibility and still log a deprecation warning.
After a while you then can remove the *deprecated_pos_args

@emanlove emanlove modified the milestones: V6.2.0, v6.3.0 Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants