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
Suggest partition() or rpartition() over specific uses of split() #4440
Comments
Sounds reasonable, I'd merge that. |
Giving this a shot |
@alanyee (CC: @Pierre-Sassoulas & @cdce8p ) Would it make sense to show a warning when |
I don't think it would make sense. In my opinion, stylistically, |
I understand, i was thinking of throwing a suggestion to use (r)partition instead of split when a user is using .split() and with a maxsplit value of Constant int 1 -- since partition accomplishes the same thing. I'll leave that check out for now based on your inputs. Thanks! |
#4469 is getting close to being finished. However, lately I was thinking if this should be an extension instead. It is a matter of preference after all. What do you think? @alanyee @Pierre-Sassoulas |
I think this can be a builtin checker as calculating only the first or last element of the list gives better performance at no cost, so this does not feel like an opinionated checker. Maybe I'm mistaken ? |
I'm 50/50 on that. So if the most think it can be builtin, we can do that. |
What would be the drawbacks ? ( |
Yeah. Might be just me, but I for example though that this would not be equal (until I tested it): l = "1,2,3"
a = l.split("-")[-1]
b = l.rpartition("-")[2]
a == b When thinking about it now, it does make sense though. With |
Ok, so if I understand correctly the problem is the suggestion we make and not the checker itself. What should be the less opinionated suggestion we can ? |
I couldn't put it in words first, but that describes it pretty much.
The could actually be a good solution! Instead of suggestion @yushao2 What do you think? |
That makes sense, it might be more intuitive as compared to suggesting partition due to the difference in return type/formats. Should I go ahead and make the necessary changes in the checker in my PR? |
Lets go that route then!
👍🏻 |
Hi guys! Just like to ask a small question -- what does builtin refer to specifically in this context? |
Hello :)
|
Just a last minute thought, as it's non opinionated and always better to use |
I just checked the difference between the two and since it's non-opinionated, I agree that it should be renamed to Is it fine if I leave the logic in this source file, |
I don't think it would be worth the time, it's still a recommendation, we just have higher confidence that it's a good one (unlike some other check where the user have to consider the situation). |
Let me make the changes then :) |
Since Pylin 2.9.0 there is new check: > New checker use-maxsplit-arg. Emitted either when accessing only the first or last element of str.split(). See pylint-dev/pylint#4440 for details.
Since Pylint 2.9.0 there is new check: > New checker use-maxsplit-arg. Emitted either when accessing only the first or last element of str.split(). See pylint-dev/pylint#4440 for details.
Since Pylint 2.9.0 there is new check: > New checker use-maxsplit-arg. Emitted either when accessing only the first or last element of str.split(). See pylint-dev/pylint#4440 for details.
Is your feature request related to a problem? Please describe
Oftentimes, I see such snippets:
Since the developer only cares about the first and/or last parts of the split, the better alternative is to use
partition()
andrpartition()
such aspartition()
andrpartition()
only splits the string once, so they become equivalent tosplit(sep=",", maxsplit=1)
andrsplit(sep=",", maxsplit=1)
. This avoids havingsplit()
split a string multiple times as a given string may have multiple copies of thesep
value in the string when the developer only cares about the first or last part of the string.Describe the solution you'd like
Within a given scope of Python code, Pylint should check for similar uses of
where
seq
is only referenced and used for the first and/or last split, and Pylint should throw a warning about a potentially unnecessary, expensive operation.Additional context
N/A
The text was updated successfully, but these errors were encountered: