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

What is the rationale behind use-maxsplit-arg? #6231

Closed
tmccall8829 opened this issue Apr 7, 2022 · 1 comment · Fixed by #6232
Closed

What is the rationale behind use-maxsplit-arg? #6231

tmccall8829 opened this issue Apr 7, 2022 · 1 comment · Fixed by #6232
Assignees

Comments

@tmccall8829
Copy link

Question

I have a python script that includes the following line:

"/my/endpoint".split("/")[-1]

When I run my script through pylint, it hits me with a use-maxsplit-arg warning and suggests I do the following instead:

"/my/endpoint".rsplit("/", maxsplit=1)[-1]

What I'm wondering is, what is the rationale behind that rule? It seems unnecessarily complex, and when I time things with timeit it looks like the recommended change actually takes slightly longer to execute than the original:

python -m timeit '"/my/endpoint".split("/")[-1]'
2000000 loops, best of 5: 124 nsec per loop

python -m timeit '"/my/endpoint".rsplit("/", maxsplit=1)[-1]'
2000000 loops, best of 5: 154 nsec per loop

Thanks!

Documentation for future user

This might already exist, but it'd be nice if there was an easy to access "rationale guide" behind some of these rules. Like I said, maybe that already exists but I wasn't able to find it quickly. I don't want to just make pylint ignore a rule because I don't understand it, but this one comes up often and I've never been able to figure out the rationale.

Additional context

No response

@tmccall8829 tmccall8829 added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Apr 7, 2022
@jacobtylerwalls
Copy link
Member

I wasn't involved in reviewing that checker, but it looks like speed was the reason. You can imagine this making a bigger difference with a larger string:

>>> search_term = 'my/endpoint'
>>> timeit("search_term.split('/')[-1]", globals=globals())
0.085186209063977
>>> timeit("search_term.rsplit('/', maxsplit=1)[-1]", globals=globals())
0.11102708312682807
>>> search_term = 'my/endpoint' * 50
>>> timeit("search_term.split('/')[-1]", globals=globals())
1.1893660409841686
>>> timeit("search_term.rsplit('/', maxsplit=1)[-1]", globals=globals())
0.1747692502103746

We're working on a documentation project to provide a page for each message. I could envision adding a sentence about this there, acknowledging that there is negligible or even worse performance impact with such small strings. I'll just open that PR to get it going.

@jacobtylerwalls jacobtylerwalls added Documentation 📗 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Apr 8, 2022
@jacobtylerwalls jacobtylerwalls self-assigned this Apr 8, 2022
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.14.0 milestone Apr 8, 2022
@Pierre-Sassoulas Pierre-Sassoulas removed this from the 2.14.0 milestone May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants