-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Formatter: Specify indentation for function parameters #8360
Comments
I don't agree with this because it's rare in the wild to all parameters starts with fullname of function/class name. |
I think this is the same as #8360 |
@MichaReiser Can you update the comment to link the correct issue? Currently, it's referring to itself ;) |
Why do I always do this... 🤦 I'm trying to find the issue again but can't for some reason. I saw it before. I'll re-open this in the meantime. |
I don't see why there is any need to downvote this, this a contentious issue based on numerous heated issues in black, etc that you can look up. The question is not whether you like it but whether there are enough who do. The proposal is not to change the default, or force one way or another, but hopefully have a configuration option so that those of use who much prefer combining the PEP-8 recommendation of an extra indent level for function args with sad-face can do so. Also, it is not just about readability in the contrived case where the args happen to match the function name, I feel the different alignment aids readbility regardless. |
@T-256 you might be missing my main point there. Even with different names, it's still not very readable:
Look at this instead (much more readable):
|
@rwightman wrote:
Thanks for highlighting this. Indeed, it was not my intention to advocate for this to become the default in any way. |
Just FYI: I downvoted this issue because I find any config for the format a negative thing, because it leads to longwinded bikesheding discussions and I really love the way black handled it: have some guiding principles (make diffs as short as possible, which means the indent should be as short as possible to not result in line breaks) and then take a decision and be done with it. The amount of time spent on discussing and/or tweaking styles (both on these issues/PRs or your own projects when you change some config) will (in my opinion) never be outweighed by any gains you have because some formatting is a tiny bit more readable than what black or now ruff does. (and yes: been there, done it, will never get the time back I spent on making intelij format SQL the way I wanted it to... And also not the time I spend on resolving merge conflicts because I thought aligned AS was a good thing...) |
Still, it's readable into my eyes, I think it's a personalization choose but perhaps modern editors with syntax highlighting solve your problem: def perform_some_tasks(
data: List[float],
arg2: Union[int, str, bool],
hello: Optional[int],
good: Dict[str, Any],
morning: float,
) -> None:
...
def perform_some_tasks(
data: List[float],
arg2: Union[int, str, bool],
hello: Optional[int],
good: Dict[str, Any],
morning: float,
) -> None:
... I disliked because I'd not like to encounter codebases with this option enabled, the reason is that going to show lines fat, and in syntax colorized editor it's uglier. (This is my taste, maybe it is different for youً!) |
Hi, While indenting function arguments one extra level may not be readable for some, it may help programmers who must use alternatives to sight to program - several screen reading technologies include a feature to announce indentation level (one of them provides an option to announce indentation with audio). For example: def my_function(): Versus: To most of us, the presence of comma at the end makes it clear that we are passing a keyword argument to this function. However, for people relying on assistive technologies (especially screen readers), programmers must infer from surrounding code (or subtle changes to output by text-to-speech engine as configured by the screen reading technology), unless they listen to speech output carefully, they may not know where function body starts and ends. A counterargument could be that blind programmers can ask the screen reader to announce all punctuation, and some would cite productivity as a reason to not take up that alternative. Another alternative is to ask people to use type hints in function definitions, but not all developers may heed that advice (type hints do inform programmers that we are reading function signature, not the body itself, quite useful if arguments are written on the same indentation as rest of the function body). I understand that what I wrote above opens up a can of possibly unnecessary discussions, but I think it is also important to think about readability from the perspective of those who may not see what we wrote and must rely on alternative modes to figure out what's going on. Thanks. |
I use a 'modern editor', PyCharm, and it defaults to the extra indent level (continuation indent 8) for formatting because it's following PEP-8. Again, the extra indent is PEP-8. Sadface is not, but I do prefer sadface in combo with this. |
I never rely on syntax highlighting because in many cases there is no syntax highlighting. Just to name a few:
|
@jankatins: I would suggest that we all refrain from making absolute statements like this (with phrases like "will never be outweighed by..."). If you can find statistics that support your "will never" claim, please feel free to share it here. (And note that the statistics need to be statistically rigorous.) Otherwise, I would suggest not making design choices that hurt people's productivity. By the way, "a tiny bit more readable" is your personal opinion, and there are a lot of people who do not agree with your opinion. |
Added a "In my opinion". I'm looking forward to the statistics why this additional config improves readability and productivity and not decreases is because of the time lost doing bike shedding. |
Whilst I'm not a fan of this style, and I'd rather rely on syntax highlighting + empty left side, this is the best counter-argument I've seen to "just use syntax highlights". Validating it for readability reasons for those scenarios. Not something I'd use in mine. But I could also work with a codebase that uses it (if autoformatted) w/o affecting my productivity. |
Hi authors of Ruff, may I follow up on this issue? Do you have plans to include this configurability into Ruff in the near future? Thanks! |
@jsh9 this is not on our near-time roadmap because there are other features with a wider reach that we want to work on. |
Thanks @MichaReiser ! Would you be willing to point to us (people who would like this feature) where this feature is implemented in the code? I'm not familiar with Rust, so it's difficult for me to contribute. But with your pointers maybe I (or other people) can take a stab at it. |
The complexity of the requested feature isn't specifically implementing it, but:
The code change itself is probably rather straightforward (I say that often and am then surprised that formatter changes are always hard 😆 ). It might be as simple as adding an extra indent level to the code in ruff/crates/ruff_python_formatter/src/other/parameters.rs Lines 249 to 282 in febc69a
write!(
f,
[
token("("),
dangling_open_parenthesis_comments(parenthesis_dangling),
soft_two_level_block_indent(&group(&format_inner)),
token(")")
]
) It would require a custom let snapshot = f.snapshot();
f.write_element(FormatElement::Tag(StartIndent));
f.write_element(FormatElement::Tag(StartIndent)); // <---- this
match self.mode {
IndentMode::Soft => write!(f, [soft_line_break()])?,
IndentMode::Block => write!(f, [hard_line_break()])?,
IndentMode::SoftLineOrSpace | IndentMode::SoftSpace => {
write!(f, [soft_line_break_or_space()])?;
}
}
let is_empty = {
let mut recording = f.start_recording();
recording.write_fmt(Arguments::from(&self.content))?;
recording.stop().is_empty()
};
if is_empty {
f.restore_snapshot(snapshot);
return Ok(());
}
f.write_element(FormatElement::Tag(EndIndent));
f.write_element(FormatElement::Tag(EndIndent)); // <---- and this
match self.mode {
IndentMode::Soft => write!(f, [soft_line_break()]),
IndentMode::Block => write!(f, [hard_line_break()]),
IndentMode::SoftSpace => write!(f, [soft_line_break_or_space()]),
IndentMode::SoftLineOrSpace => Ok(()),
} |
I am watching this issue more than 1 month, and I always wait for implement this function in ruff. our company have some code style rules, All other rules can be satisfied through configuration, only this one I can't. This style also follow pep8 style. I like ruff, but I can't directly using it in my work and life. |
I'd very much appreciate this feature. I'll get bullied into using Pycharm's formatter otherwise 😅 |
Hi @MichaReiser , May I follow up on this topic here? Have you and/or the Ruff team discuss these questions that you outlined above?
Thank you! |
No, I'm sorry. We haven't decided on this yet |
This comment was marked as spam.
This comment was marked as spam.
Brackets are sufficient for readability in both cases above. So if readability is the goal then the first option is newline character before closing bracket. And the second option is complication of indentation logic by double indentation levels specifically for arguments lists. If complicating logic considered pythonic option, then maybe pep8 should allow both cases explicitly. |
Google's |
Yes, also following this. Hope it will become adopted. |
The current indentation behavior for function parameters is: 4-space indentation at function definitions, like this:
This leads to reduced readability, because the function name and the argument names become harder to distinguish.
Adding one more level of indentation greatly improves readability:
PEP8 also has the same suggestion: https://peps.python.org/pep-0008/#indentation
Originally posted by @jsh9 in #7310 (comment)
The text was updated successfully, but these errors were encountered: