182

I have a line of the following code (don't blame for naming conventions, they are not mine):

subkeyword = Session.query(
    Subkeyword.subkeyword_id, Subkeyword.subkeyword_word
).filter_by(
    subkeyword_company_id=self.e_company_id
).filter_by(
    subkeyword_word=subkeyword_word
).filter_by(
    subkeyword_active=True
).one()

I don't like how it looks like (not too readable) but I don't have any better idea to limit lines to 79 characters in this situation. Is there a better way of breaking it (preferably without backslashes)?

Juliusz Gonera
  • 4,658
  • 5
  • 32
  • 35

9 Answers9

320

You could use additional parentheses:

subkeyword = (
        Session.query(Subkeyword.subkeyword_id, Subkeyword.subkeyword_word)
        .filter_by(subkeyword_company_id=self.e_company_id)
        .filter_by(subkeyword_word=subkeyword_word)
        .filter_by(subkeyword_active=True)
        .one()
    )
sth
  • 222,467
  • 53
  • 283
  • 367
  • 1
    I also like it best. Doesn't add more code and it's without backslashes. – Juliusz Gonera Jan 22 '11 at 23:17
  • 26
    Not sure what justifies the extra indentation here; I think this solution reads just as well with the hanging lines indented just once and the trailing paren not at all. – Carl Meyer Jun 23 '15 at 21:57
  • 5
    In my opinion double indentation is useful here because it is visually distinct from a normal indented block. When surrounded by other code this makes it more obvious that it is a wrapped single line. – sth Oct 21 '16 at 19:57
  • 1
    Best answer, in terms of using parens. As mentioned in a comment by Shanimal in another answer, using implied line continuation via parentheses is actually [PEP 8 preferred](https://www.python.org/dev/peps/pep-0008/) vs the continuation character `\` – kevlarr Oct 12 '17 at 15:49
  • I prefer backslashes. Parenthesis is not a hint for all situation. As an example, it does not work with assignment operator. Imagine you want to break lines in this chain: `foo.set_default('bar', {}).set_default('spam', {}).set_default('eggs', {})['lol'] = 'yeah'` – loutre Jul 09 '18 at 14:20
  • 1
    Another advantage is that you can comment each line if needed, which doesn't work for the `\\` method. – David Contreras Aug 01 '22 at 19:27
  • In late 2022 I still like this solution the best and it seems that this is also what [Black](https://github.com/psf/black) transforms the code from my original question to (minus the extra indentation mentioned by @CarlMeyer). In general, these days it's worth to just use a leading code formatting tool in your language of choice and stop worrying about manual formatting. – Juliusz Gonera Dec 02 '22 at 22:22
71

This is a case where a line continuation character is preferred to open parentheses. The need for this style becomes more obvious as method names get longer and as methods start taking arguments:

subkeyword = Session.query(Subkeyword.subkeyword_id, Subkeyword.subkeyword_word) \
                    .filter_by(subkeyword_company_id=self.e_company_id)          \
                    .filter_by(subkeyword_word=subkeyword_word)                  \
                    .filter_by(subkeyword_active=True)                           \
                    .one()

PEP 8 is intend to be interpreted with a measure of common-sense and an eye for both the practical and the beautiful. Happily violate any PEP 8 guideline that results in ugly or hard to read code.

That being said, if you frequently find yourself at odds with PEP 8, it may be a sign that there are readability issues that transcend your choice of whitespace :-)

Just a student
  • 10,560
  • 2
  • 41
  • 69
Raymond Hettinger
  • 216,523
  • 63
  • 388
  • 485
  • 2
    +1 on backslashes and aligning the chained filters in this particular case. This situation arises in Django as well and is most readable this way -- but in _any_ other situation I feel like parenthesized phrases are superior (don't suffer from the "is there whitespace after my backslash?" problem). That said, parenthesizing the phrase can be used to achieve the same effect -- but it puts you in Lisp reading mode in the middle of reading Python, which I find jarring. – zxq9 May 31 '13 at 01:02
  • 16
    I don't see how this solution is better able to cope "as method names get longer and as methods start taking arguments" than either the "wrap in outer parens" or "line-break after each open paren and before each close paren" solutions. In fact it's worse at handling that, since (at least as shown here) it requires a much deeper indent for every hanging line. – Carl Meyer Jun 23 '15 at 21:57
  • 2
    Far too much indent for the filter calls. One tab or 4 spaces would have been enough here. Also alignment of the `\` ... How many seconds did you hold down that space key? Generally I am against all ways, which require you to hammer that space key like there is no tomorrow. – Zelphir Kaltstahl Nov 25 '16 at 16:05
  • 5
    fwiw, PEP8 reads "The preferred way of wrapping long lines is by using Python's implied line continuation inside parentheses, brackets and braces. Long lines can be broken over multiple lines by wrapping expressions in parentheses. These should be used in preference to using a backslash for line continuation." —[Python.org](https://www.python.org/dev/peps/pep-0008/#indentation) It goes on to discuss when backslashes may be appropriate – Shanimal Aug 10 '17 at 18:28
  • 2
    Great reference to PEP8! An annoying issue here with aligning all the `.filter` calls is that if you change `subkeyword` to `sub_keyword`, you now have to fix the indentation of **every single line** just because you changed the variable name. Not good when style actually hinders maintainability... – kevlarr Oct 12 '17 at 15:45
  • The alignment in this form also fails if the LHS expression is too long, like `my_obj["long_keyword_name"].attrs["another_long_name"] = my_obj["long_keyword_name"].attrs["another_long_name"].replace("aaa", "bbb").replace("ccc", "ddd").replace("eee", "fff")`. – gerrit Jan 05 '18 at 11:08
  • Ugh. Backslashes are brittle. – Vicki B Jul 22 '19 at 22:01
21

My personal choice would be:

subkeyword = Session.query(
    Subkeyword.subkeyword_id,
    Subkeyword.subkeyword_word,
).filter_by(
    subkeyword_company_id=self.e_company_id,
    subkeyword_word=subkeyword_word,
    subkeyword_active=True,
).one()
pkoch
  • 2,682
  • 1
  • 19
  • 17
  • 2
    I agree if there are several parameters being passed in but it looks ugly when 0 or 1 parameters are common. For example: https://gist.github.com/andybak/b23b6ad9a68c7e1b794d – Andy Baker Jul 05 '15 at 11:50
  • 2
    Yeah, that style has degenerate cases (like any style). I would not break on all the open parens. None of this leaves me happy, but here are some cases: https://gist.github.com/pkoch/8098c76614765750f769 – pkoch Jul 30 '15 at 20:57
12

Just store the intermediate result/object and invoke the next method on it, e.g.

q = Session.query(Subkeyword.subkeyword_id, Subkeyword.subkeyword_word)
q = q.filter_by(subkeyword_company_id=self.e_company_id)
q = q.filter_by(subkeyword_word=subkeyword_word)
q = q.filter_by(subkeyword_active=True)
subkeyword = q.one()
Ivo van der Wijk
  • 16,341
  • 4
  • 43
  • 57
  • 12
    This works well for something like a query but as a general pattern, I'm not so sure. For example, when chaining in Beautiful Soup like `team_members = soup.find(class_='section team').find_all('ul').find_all('li')`, the return value from each `.find(...)` call doesn't fit the meaning of `team_members` yet. – Taylor D. Edmiston Oct 20 '16 at 19:52
  • 2
    @TaylorEdmiston You can have different names for the partial results of course. Something like `section = soup.find(class_='section team')` and `team_members = section.find_all('ul').find_all('li')`. – Jeyekomon Jul 08 '19 at 13:51
9

It's a bit of a different solution than provided by others but a favorite of mine since it leads to nifty metaprogramming sometimes.

base = [Subkeyword.subkeyword_id, Subkeyword_word]
search = {
    'subkeyword_company_id':self.e_company_id,
    'subkeyword_word':subkeyword_word,
    'subkeyword_active':True,
    }
subkeyword = Session.query(*base).filter_by(**search).one()

This is a nice technique for building searches. Go through a list of conditionals to mine from your complex query form (or string-based deductions about what the user is looking for), then just explode the dictionary into the filter.

  • 2
    Great solution. ️ Although not explicitly answering the question, it reveals how programmers sometimes try to solve _pain_ & problems (here: unreadable filter-chain) they don't need to have. This _pain_ is a symptomatic signal of a deeper cause (often conceptually, structurally, design issues, even naming). E.g. your dict `search` has the meaningful purpose of a `predicate` or `matcher`, conceptually like [QBE](https://en.wikipedia.org/wiki/Query_by_Example#As_a_general_technique): It's matching all attributes) like in SQL `WHERE .. AND ..`. – hc_dev Sep 29 '21 at 20:34
4

According to Python Language Reference
You can use a backslash.
Or simply break it. If a bracket is not paired, python will not treat that as a line. And under such circumstance, the indentation of following lines doesn't matter.

Haozhun
  • 6,331
  • 3
  • 29
  • 50
1

I like to indent the arguments by two blocks, and the statement by one block, like these:

for image_pathname in image_directory.iterdir():
    image = cv2.imread(str(image_pathname))
    input_image = np.resize(
            image, (height, width, 3)
        ).transpose((2,0,1)).reshape(1, 3, height, width)
    net.forward_all(data=input_image)
    segmentation_index = net.blobs[
            'argmax'
        ].data.squeeze().transpose(1,2,0).astype(np.uint8)
    segmentation = np.empty(segmentation_index.shape, dtype=np.uint8)
    cv2.LUT(segmentation_index, label_colours, segmentation)
    prediction_pathname = prediction_directory / image_pathname.name
    cv2.imwrite(str(prediction_pathname), segmentation)
acgtyrant
  • 1,721
  • 1
  • 16
  • 24
1

You seems using SQLAlchemy, if it is true, sqlalchemy.orm.query.Query.filter_by() method takes multiple keyword arguments, so you could write like:

subkeyword = Session.query(Subkeyword.subkeyword_id,
                           Subkeyword.subkeyword_word) \
                    .filter_by(subkeyword_company_id=self.e_company_id,
                               subkeyword_word=subkeyword_word,
                               subkeyword_active=True) \
                    .one()

But it would be better:

subkeyword = Session.query(Subkeyword.subkeyword_id,
                           Subkeyword.subkeyword_word)
subkeyword = subkeyword.filter_by(subkeyword_company_id=self.e_company_id,
                                  subkeyword_word=subkeyword_word,
                                  subkeyword_active=True)
subkeuword = subkeyword.one()
minhee
  • 5,688
  • 5
  • 43
  • 81
  • +1 for SQLAlchemy filter_by() hint. It's good for this example, but I often use filter() instead which accepts only 1 condition. – Juliusz Gonera Jan 22 '11 at 23:18
1

A slight variation of the top answer: the main object (Session) is kept in the top line and a single indentation is used. This enables quick identification of the main object and all subsequent chained method calls..

subkeyword = (Session
    .query(Subkeyword.subkeyword_id, Subkeyword.subkeyword_word)
    .filter_by(subkeyword_company_id=self.e_company_id)
    .filter_by(subkeyword_word=subkeyword_word)
    .filter_by(subkeyword_active=True)
    .one()
)
dojuba
  • 2,199
  • 1
  • 18
  • 16