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

Unexpected keyword 'maxsplit', for call to method split(sep, maxsplit = None) of 'string' #9998

Closed
vam-google opened this issue Oct 11, 2019 · 8 comments
Labels
P1 I'll work on this now. (Assignee required) type: bug

Comments

@vam-google
Copy link
Collaborator

vam-google commented Oct 11, 2019

Description of the problem / feature request:

The maxsplit argument in string.split() method is a positional argument now, but it is still documented as split(sep, maxsplit = None).

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Call:

"a=b".split("=", maxsplit = 1) 

anywhere in Starlark code. It works for bazel 0.28.1 and below, but the following for bazel 1.0.0:

unexpected keyword 'maxsplit', for call to method split(sep, maxsplit = None) of 'string'`

The documentation is not updated and it is a breaking change for 1.0.0

@vam-google
Copy link
Collaborator Author

Ah, whatever, not worth the trouble, probably...

@jmillikin-stripe
Copy link
Contributor

This is breaking the Kubernetes build (at https://github.com/kubernetes/kubernetes/blob/v1.16.2/build/code_generation.bzl#L44):

return go_prefix + "/" + pkg.replace("staging/src/", "vendor/", maxsplit = 1)

cc @dslomov (release shepherd of v1.0)

@jin jin reopened this Oct 16, 2019
@jin jin added team-Starlark untriaged P1 I'll work on this now. (Assignee required) and removed untriaged labels Oct 16, 2019
@jin
Copy link
Member

jin commented Oct 16, 2019

@laurentlb

@meisterT
Copy link
Member

I assume #8147 is related cc @c-parsons

@laurentlb
Copy link
Contributor

fyi @alandonovan
"a=b".split("=", maxsplit = 1) is allowed in Python 3 and rejected in Python 2.

I think maxsplit was marked as positional-only in Bazel by accident.

@gregestren
Copy link
Contributor

What's the right priority for this?

@alandonovan
Copy link
Contributor

alandonovan commented Jul 7, 2020

I think this is the resolution of #8147 working as intended.

As with Python, nearly all parameters of Starlark built-in functions are positional only; the documentation promises to be explicit in every case where named arguments are permitted, but falls slightly short. The true list is: int(x, base), fail(sep), min(key), max(key), sorted(key, reverse). I'm not sure why Python3 decided to permit split(maxsplit) to be named where Python2 rejected it---perhaps passing a second separator string for maxsplit was a frequent mistake? (Conversely, Python2 accepts int(x=1) but Python3 rejects it.) I don't think it's worth adding maxsplit back.

See also go.starlark.net: google/starlark-go#292

@alandonovan
Copy link
Contributor

Let's re-open this as a documentation issue.

The task is to ensure that the starlark spec makes plain that all core builtins reject named arguments, except for the following: int(x, base), fail(sep), min(key), max(key), sorted(key, reverse).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) type: bug
Projects
None yet
Development

No branches or pull requests

7 participants