yapf formats long call chains with weird spacing

[qv-pat]

Take the following lengthy call chain

def some_function(session):
    result = (session.query(models.Employee.id).filter(models.Employee.business_id == unique_id).filter(models.Employee.age > 25).order_by(models.Employee.id.asc()).all())

I'm would love an output that looked something like this:

def some_function(session):
    result = (
        session.query(models.Employee.id)
        .filter(models.Employee.business_id == unique_id)
        .filter(models.Employee.age > 25)
        .order_by(models.Employee.id.asc())
        .all()
    )

But instead yapf is doing this:

def some_function(session):
    result = (
        session.query(models.Employee.id
                      ).filter(models.Employee.business_id == unique_id
                               ).filter(models.Employee.age > 25
                                        ).order_by(models.Employee.id.asc()
                                                   ).all()
    )

I've tried everything I can think of

• CONTINUATION_ALIGN_STYLE
• SPLIT_BEFORE_DOT
• SPLIT_BEFORE_EXPRESSION_AFTER_OPENING_PAREN
• Adjusting the SPLIT_PENALTY_AFTER_OPENING_BRACKET value

It wouldn't be so bad if it didn't change my formatting from that second code block to the third. Right now the only thing I can think of is to either wrap the blocks with #yapf: disable tags or to use back slashes, both of which I'm pretty against.

yapf==0.24.0

[joshburkart]

I tend to work around this by putting in fake comments:

a = (  #
    session.query(...)  #
    .filter(...)  #
    .filter(...)  #
    ...
)

[carlthome]

:+1:

In TensorFlow's ETL API tf.data a chain of method calls is very common. It would be great to see YAPF give a nice default behavior for such fluent interfaces.

Example

Before YAPF

def create_dataset(metadata: pd.DataFrame) -> tf.data.Dataset:
    dataset = (
        tf.data.Dataset
        .from_tensor_slices(dict(metadata))
        .map(load, 256)
        .map(collate)
        .shuffle(256)
        .batch(32, drop_remainder=True)
        .prefetch(1)
    )
    return dataset

After YAPF (yapf==0.28.0 defaults)

def create_dataset(metadata: pd.DataFrame) -> tf.data.Dataset:
    dataset = (tf.data.Dataset.from_tensor_slices(dict(metadata)).map(
        load,
        256).map(collate).shuffle(256).batch(32,
                                             drop_remainder=True).prefetch(1))
    return dataset

[sylann]

It is possible to write chains like this to prevent yapf from changing the indentation:

def some_function(session):
    result = session.query(models.Employee.id) \
        .filter(models.Employee.business_id == unique_id) \
        .filter(models.Employee.age > 25) \
        .order_by(models.Employee.id.asc()) \
        .all()

Similar to the funny usage of empty comments above. (Note that the space before the \ is not necessary at all, I just find it more readable.)

However, I still prefer the way the op asked in the first place, because it's easier to arrange and in some cases, you could put a call chain as an argument to another function call.

session.query(
    session.query(models.Employee.id)
    .filter(models.Employee.business_id == unique_id)
    .filter(models.Employee.age > 25)
    .order_by(models.Employee.id.asc())
    .exists()
).scalar()

In this case, the backslash trick from above wouldn't be allowed, (contrary to the empty comment trick), and yapf will reformat like this:

session.query(
    session.query(models.Employee.id).filter(models.Employee.business_id == unique_id
                                             ).filter(models.Employee.age > 25
                                                      ).order_by(models.Employee.id.asc()).exists()
).scalar()

I know black deals with this syntax "properly", but it's terribly off on many other things, so I doubt i'll ever use it. Anyone knows if it's possible that Yapf tackles this issue? I would like to start using a formatter.

[BenC14]

For reference black had this figured out in 2018 feels like something that should be brought to YAPF at least as an option. https://github.com/psf/black/issues/67#issuecomment-389681164

[bwendling]

YAPF definitely doesn't have good support for long calling chains (as you guys have noticed). Basically, YAPF doesn't identify long calling chains well. It's kind of tricky in general (there's some really heinous code that deals with these things). The best way to deal with this in YAPF is to identify individual calls and give them a higher "split cost" that the stuff around them. It's theoretically possible, just hasn't been done...