PEP 736: Shorthand syntax for keyword arguments at invocation

PEPs
103

If there are 10 named arguments to forward that way, you’d add 10 times the same lone keyword in the function invocation?

1 Like
104

Here’s an anecdote. Last night I was looking at this code from a library:

def instrument_app(
    app: fastapi.FastAPI,
    server_request_hook: _ServerRequestHookT = None,
    client_request_hook: _ClientRequestHookT = None,
    client_response_hook: _ClientResponseHookT = None,
    tracer_provider=None,
    meter_provider=None,
    excluded_urls=None,
):
    ...
    if excluded_urls is None:
        excluded_urls = _excluded_urls_from_env
    else:
        excluded_urls = parse_excluded_urls(excluded_urls)
    meter = get_meter(
        __name__,
        __version__,
        meter_provider,
        schema_url='https://opentelemetry.io/schemas/1.11.0',
    )
    app.add_middleware(
        OpenTelemetryMiddleware,
        excluded_urls=excluded_urls,
        default_span_details=_get_default_span_details,
        server_request_hook=server_request_hook,
        client_request_hook=client_request_hook,
        client_response_hook=client_response_hook,
        tracer_provider=tracer_provider,
        meter=meter,
    )

You don’t have to understand it. Just know that as I was reading this I thought that I was seeing basically all the code that I cared about. I didn’t care what exactly parse_excluded_urls or get_meter did. So I thought that this was doing little more than passing the arguments of instrument_app to app.add_middleware.

This was surprising to me, as I had reason to expect more. I pointed this out to coworkers, and one helpfully showed me what I was missing.

This morning I discovered this PEP. And I thought back to last night and realised that if the code was written with this shorthand, I would have immediately seen what was previously more like a needle in a haystack:

    app.add_middleware(
        OpenTelemetryMiddleware,
        excluded_urls=,
        default_span_details=_get_default_span_details,
        server_request_hook=,
        client_request_hook=,
        client_response_hook=,
        tracer_provider=,
        meter=,
    )

_get_default_span_details contained the extra information that I was looking for.

More generally, I think this shorthand achieves what I consider a common benefit of keeping code DRY: when you remove duplication, important differences become more visible.

16 Likes
105

That’s cool to see, and was the main takeaway I got from the article @boxed posted.

OTOH, one of the more convincing (to me) counterarguments is that it “discourages context-appropriate variable names”, as @pf_moore mentioned above. To see how that would apply to OpenTelemetry’s API I would have to understand it better, but for starters the name _get_default_span_details suggests a callable, and default_span_details suggests a container of some sort. If instead the line were:

default_span_details=_get_default_span_details()

it seems probable that you would have spotted it’s uniqueness without the PEP 736 syntax. If, on the other hand, OpenTelemetry called the variable default_span_details, the needle would remain buried, as the PEP736 syntax would elide it just like the other variables.

1 Like
106

That is pretty expected after the acceptance of PEP 736. They will rename it.

107

If instead the line were:
default_span_details=_get_default_span_details()
it seems probable that you would have spotted it’s uniqueness without the PEP 736 syntax

I don’t see how this would make much difference:

app.add_middleware(
        OpenTelemetryMiddleware,
        excluded_urls=excluded_urls,
        default_span_details=_get_default_span_details(),
        server_request_hook=server_request_hook,
        client_request_hook=client_request_hook,
        client_response_hook=client_response_hook,
        tracer_provider=tracer_provider,
        meter=meter,
    )

This seems just as easy to overlook as before to me. Maybe it’s better if your syntax highlighting makes () have very different colors, but that seems very specific to this case anyway.

108

Assuming that PEP 736 would capture variables in all scopes, both the old style and new style would be completely equivalent (unless there’s something I’ve completely missed.)

func(
    foo=foo,
    bar=bar,
)
# Equivalent to
func(
    foo=,
    bar=,
)

Now we have two ways to create the same thing, and it’s seemingly a stylistic[1] choice whether one is better than the other. That means there’s another thing for developers to argue about. While that in and of itself is not an argument against the PEP, I think the PEP should also include a change to PEP 8 to make one of these choices the preferred style. That would reduce the amount of argument, both IRL and in various projects’ issue trackers.

  1. For a quick and dirty script it might not just be a stylistic but an ergonomic choice as well. ↩︎

109

I disagree. I suspect I would use the style once my minimum Python hit a supporting version, but I would really rather not add the weight of PEP8 (which is frequently misapplied) behind it. I’m sure there’ll be a flake8 plugin and ruff support, which should be enough. If projects want to consistently use/avoid it, they can set that policy in their linter/formatter config.

5 Likes
110

That’s fair. My point is that examples like this show when eliding a kwarg illuminates an important name. We should also consider whether the API could’ve used more context-appropriate names in the first place, and how PEP-736 would look with those names.

In this case, it seems like the function does have more context on meter and excluded_urls (and maybe more, depending upon what’s in the …), which could use a more context-appropriate name. This is a case where the poster’s missing context was only missed because it came from the global scope, and only stood out so much in PEP 736 because the add_instrument didn’t use a more context-appropriate name for meter and excluded_urls. Maybe this is the appropriate comparison?

app.add_middleware(
        OpenTelemetryMiddleware,
        excluded_urls=parsed_exclusions,
        default_span_details=_get_default_span_details,
        server_request_hook=,
        client_request_hook=,
        client_response_hook=,
        tracer_provider=,
        meter=instrument_meter,
    )

…which still stands out. So the more I think about this example, the more I agree with it’s author. Wrappers and factories like instrument_app() will usually only have more context for some subset of the arguments they pass along, which are exactly the arguments that would stand out if the others are elided.

So to paraphrase the author, the PEP makes important differences become more visible so long as the important differences are given different names.

2 Likes
111

No, but it was mentioned in the discussion thread, and I think it should be mentioned along with the other rejected forms in the PEP. (Plus, I did mention that Nix allows {inherit x y; ...} instead of {inherit x; inherit y; ...}, and I would recommend the same in Python if I were advocating for it.)

112

I posted more extensively about this in the other thread, but I really hope linters don’t pick this up and start pushing it. There are reasons to avoid configuring linting tools to have non-default behavior, in that it helps you keep disparate codebases uniform.

Rubocop enforces this style for Ruby, which results in this being applied in lots of trivial cases which you could argue are not harmed but are certainly not helped by the change. A lot of

foo(x: bar, y:)

To me, the end result is non-uniformity of style within a codebase, and sometimes within a single call site, for no beneficial purpose in many cases. (To clarify, I’m not saying that supporting this feature has no benefit, but rather that linter enforcement is not providing any benefit in many cases.)

So Rubocop “forces” me to write in this way, even though I’d rather not. I hope not to have the same experience with Python linters.

Regarding the above example of this revealing the intent of an otherwise obscure parameter in a long list of passthroughs, I have two thoughts.

One is that the syntax itself helps highlight the difference, as noted.

The other is that this seems like a case in which a judiciously placed comment could also have helped.

3 Likes
113

It seems like the more important detail in this code is meter, which is de-emphasized in the new version. That’s the non-default argument that’s being added by this function, right?

Maybe that wasn’t relevant to you in the specific case you were debugging, but I would think it’s the primary reason for this method.

1 Like
114

I think the end result is going to be that linters and autoformatters pick this up eventually, which is why I think it’s better to add it to PEP 8 immediately to reduce the amount of discussion around this topic. But I see that many disagree.

To me, the PEP is about adding an additional style of function call, and I think it’s not great to add a new style unless it’s not the one being pushed as correct style.

115

But PEP 8 is not a general style guide, it’s a style guide for the stdlib itself. Popular formatting tools do not follow PEP 8.

It only makes sense to add it there if the core developers want to adopt this style in future code.

5 Likes
116

Precisely. We’ll add it into our style guide when we need it (and more likely than not, it’ll start off with “don’t change existing uses to use the new style”, which is probably not what you want to be suggesting in the PEP proposing the feature).

It’s fine to have a section of recommendations for linters/formatters though. PEP 8 isn’t that, but PEP 736 can be for this feature.

2 Likes
117

A recommendation in the PEP would satisfy me.

118

A recommendation in the PEP that linters explicitly don’t enforce this style over the traditional form would satisfy me.

(To make the point in a more balanced way, I think there are opinions on both sides, and the PEP should either take no stance on linters, or should explicitly note that there is no consensus and therefore linters should not prefer one form over the other).

13 Likes
119

I would be curious to hear from a black maintainer about how they would view this PEP as written. Would it be ignored, so long as it is possible to preserve whether the value was present, or would it be in the spirit of black to pick one and just enforce it? Would the inclusion of recommendations for linters/formatters in the PEP text have any effect on that decision?

120

I am a maintainer of Black. I would want Black to enforce PEP 736 style where possible.

That means I disagree with @pf_moore just above. If we don’t think that linters should recommend PEP 736 style, then I don’t see why we’d add the feature to the language.

2 Likes
121

To be clear, “recommend PEP 736 style” means “we will delete code that the user wrote”?

I read Paul’s preference as “correctly format both x=x and x=, but don’t change one to the other”, which seems reasonable to me.

Until refactoring tools are able to correctly handle a x= in the case where you either rename the x parameter or the x local (which obviously doesn’t rename both simultaneously), automatically deleting the variable in x=x feels likely to be more inconvenient than anything else.

13 Likes
122

Precisely. If black (or any other formatter) rewrote x=x as x=, I would do one of the following on any project I maintain:

  1. Stop using that formatter.
  2. Disable that rewrite rule, if the formatter gives me a way of doing so.
  3. Put # fmt: off around all function calls that might be affected.
  4. Make it a requirement that when passing a variable to a keyword argument, the variable must always have a different name than the keyword, to prevent this behaviour.

Of these, only (1) and (2) seem like they would be feasible in practice.

3 Likes