Skip to content

Conversation

@charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Oct 20, 2023

Summary

When analyzing:

if "root" not in (
    long_tree_name_tree.split("/")[0]
    for long_tree_name_tree in really_really_long_variable_name
):
    msg = "Could not find root. Please try a different forest."
    raise ValueError(msg)

We missed that the generator expression is parenthesized, because the parentheses are part of the generator -- so is_expression_parenthesized returns False. We needed to take into account that generators and tuples may or may not be parenthesized when determining whether we can omit parentheses while splitting an expression.

Closes #8090.

Test Plan

No changes in similarity.

Before:

project similarity index total files changed files
cpython 0.75803 1799 1647
django 0.99983 2772 34
home-assistant 0.99953 10596 186
poetry 0.99891 317 17
transformers 0.99966 2657 330
twine 1.00000 33 0
typeshed 0.99978 3669 20
warehouse 0.99977 654 13
zulip 0.99970 1459 22

After:

project similarity index total files changed files
cpython 0.75803 1799 1647
django 0.99983 2772 34
home-assistant 0.99953 10596 186
poetry 0.99891 317 17
transformers 0.99966 2657 330
twine 1.00000 33 0
typeshed 0.99978 3669 20
warehouse 0.99977 654 13
zulip 0.99970 1459 22

pub(crate) fn is_tuple_parenthesized(tuple: &ExprTuple, source: &str) -> bool {
let Some(elt) = tuple.elts.first() else {
return false;
return true;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An empty tuple has to be parenthesized. Otherwise, it... can't exist?

if visitor.max_precedence == OperatorPrecedence::None {
true
} else if visitor.pax_precedence_count > 1 {
} else if visitor.max_precedence_count > 1 {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was a typo, but please correct me if I'm wrong.

@charliermarsh charliermarsh added bug Something isn't working formatter Related to the formatter labels Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working formatter Related to the formatter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Black deviation with if x in some comprehension

3 participants