flake8-commas
flake8-commas copied to clipboard
PyCQA
Ignore C812 when only one function argument
I'd like to ignore the error when there's only one argument to the function. For example:
logger.debug(
"Really long string "
"implicitly joined"
)
Why? The purpose of the trailing comma here is to cut down on diff noise in the future. It is totally plausible that some code passes one parameter when first written and then 3 in the iteration.
I am all for trailing commas to reduce diff noise, thank you for developing this plugin! 😍 When we were trying to bring it into our flake8 setup, its flagging some cases similar to the above which I would not expect. For example when building some Django ORM queries we have code like:
intersting_pks = set(
model.foreign_models
.filter(some_criteria=True)
.values_list("pk", flat=True) # flags C812
)
I realise it would be valid syntax (in 3.7 at least) to have the , there at the end, but it just looks odd. Additionally, it does not really prevent diff churn/noise since it would not be valid to add another entry on the line following the comma.
Do you think it is feasible for the linter to allow for such syntax without having to add noqa or ignore C812? Happy to help contribute to such a change if you think it is reasonable.
What would the algorithm be @9thbit ? Because in your example if I replace set with an arbitrary function it becomes much less obvious that a comma does not make sense there.
BTW: I am not involved in this project, only another happy user. It is my hope that asking questions here will lead to the right outcome.
Absolutely, or even removing set there would be justification for both with and without a the comma…
To be honest I'm not sure I'm not familiar with how this is checked currently or what the solution is. I would probably only expect trailing commas in things that are "list"-like, maybe is it just as simple as if statements on previous lines have trailing commas – that the same pattern should be maintained for the last item. 🤔
Give the code a read, it is fairly clear I think.
Right now there is minimal state gathering. I could see a more complicated version that differentiated between one argument functions and multi argument functions. Then you could ignore the one argument error if you liked. That is one solution. I am not sure at the moment how to determine the number of arguments a function requires but it should be possible to look at the parse tree and see how many are being passed in currently.
A place where the current checker does something weird is requiring a comma after "kwargs" which is definitely not going to see more arguments after it in most code.
A place where the current checker does something weird is requiring a comma after "kwargs" which is definitely not going to see more arguments after it in most code.
black also requires a trailing comma after **kwargs (in py3.6+ only)
My opinion on this matter is that it's not about differentiating between 1 or more arguments, but more about the difference between fixed or changing argument counts.
If a function call requires two arguments, period, then I almost feel like the trailing comma hints at a possibility of giving more. But doing so would cause an immediate error, which makes me feel like a trailing comma should be at least a warning and certainly not a recommendation.
Yes, of course, the function signature could some day change allowing (maybe even requiring) one more argument. But I feel that this would be such a profound impact that reducing the diff noise by one line is probably not going to matter...
While I like this, I have also started using a "workaround" that also helps identify the string (or long bit) as a single argument. I'd much rather see #58 implemented.
The below doesn't throw any errors. It works out nicely for "segregating" arguments. I believe it also works for implicit concatenation, but I don't use that in my code.
logger.debug(
(
"Really long string " +
"explicitly joined"
),
)
I agree, this would be nice. There are cases where you know it's incredibly unlikely that a function is going to get another argument, so it just looks a bit ugly there.
