Skip to content
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

Guards on comprehensions #17097

Closed
wants to merge 1 commit into from
Closed

Guards on comprehensions #17097

wants to merge 1 commit into from

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Jun 25, 2016

Ref: #550

Note this is only tests and documentation.

👉 @JeffBezanson

@JeffBezanson
Copy link
Member

Doesn't the if usually go after the for? Or should we allow both?

@quinnj
Copy link
Member Author

quinnj commented Jun 26, 2016

I think the consensus in the issue was that before the for loop was clearer and would be the only version allowed.

@kmsquire
Copy link
Member

I vaguely remember that conversation. Just curious if there is precedent for the guard before the for? Not that there has to be, I've just never seen it.

@kmsquire
Copy link
Member

(Looking at #550 again, I don't dislike the if before the for...)

@rfourquet
Copy link
Member

I wouldn't call it a consensus yet. I would love to have this, but with the if after the for! For reference, some discussion regarding filtered comprehensions also happened at #16389. In particular I found this convincing (filtering acts on the itreration part, not on the value part of the comprehension). Also, if multiple for are implemented as suggested here, having the if at the end seems to allow more general constructs [f(i,j) for i in r1 if pred(i) for j in r2(i) if pred(i, j)]. This is how it works in python, and the mental model is very simple, it translates directly to for loops and if blocks:

for i in r1
    if pred(i) 
        for j in r2(i)
            if pred(i, j)
                yieldvalue(f(i, j))
            end 
        end 
    end
end

@StefanKarpinski
Copy link
Member

For me the most convincing argument is that this is how Python does it so it will cause the least confusion for people switching languages.

@quinnj
Copy link
Member Author

quinnj commented Jun 26, 2016

Personally I've always found the python syntax very disruptive when reading code. You read this entire comprehension/generator and then get surprised at the end by a guard. You then have to go back and re-read everything to make sure you understand what's going on. I mean, there's a reason we don't have

begin
    do_this()
    do_that()
end if surprise_condition

Putting right before the iterator also makes it more obvious IMO that its operating on the iterator.

@johnmyleswhite
Copy link
Member

A weak +1 for putting if before the for since the whole thing is a backwards for-loop to begin with. Would be stronger, except for Python precedent.

@rfourquet
Copy link
Member

FWIW, Haskell does it like Python too (or Python like Haskell?)

@JeffBezanson
Copy link
Member

Either approach is fully general, just a matter of whether the loops and guards appear inside-out or outside-in. The fact that the expression comes first does seem to imply that the whole thing should be inside-out. However I find the precedent from multiple languages slightly more convincing. Reversing the order of something tends to be mind-bending.

@ararslan
Copy link
Member

At least for me, the precedent from other languages doesn't mean much since Julia is something new and is in a position to set precedent. As such, I don't find "we should do this because Python/Ruby/etc. does this" very convincing. We have the opportunity to get it right where other languages got it wrong. 😉

@StefanKarpinski
Copy link
Member

I mean I do generally agree, but you have to pick and choose your battles.

@Sacha0
Copy link
Member

Sacha0 commented Jun 27, 2016

A weak +1 for putting if before the for since the whole thing is a backwards for-loop to begin with.

.

Either approach is fully general, just a matter of whether the loops and guards appear inside-out or outside-in. The fact that the expression comes first does seem to imply that the whole thing should be inside-out. [...] Reversing the order of something tends to be mind-bending.

I find these arguments convincing only when viewing generators/comprehensions as syntactic sugar for an imperative construct, whereas generators/comprehensions generally strike me as a declarative construct mirroring standard mathematical set notation. For example, [x^2 for x in 1:10] and (x^2 for x in 1:10) mirror {x^2 | x in 1:10}. Taking this view, guards naturally appear to the right of for. To illustrate, in standard mathematical set notation you might write a set of rationals {p/q | p in P, q in Q, q != 0} where P and Q are integer sets, e.g. P = 1:10 and Q = -5:5 in Julia notation. If generators/comprehensions are declarative constructs mirroring this notation, then the corresponding Julia generator/comprehension syntax would be [p/q for p in P, q in Q if q != 0] modulo if potentially being something else like where. I presume both Python and Haskell derive their syntax from this view.

Additionally, if one views generators/comprehensions as syntactic sugar for an imperative construct and simultaneously finds order-reversal mind-bending, doesn't consistency demand the syntax [for x in 1:10 x^2], or with a guard [for x in 1:10 if x != 1 x^2]? Doesn't the existing ordering of the implicit-arguments lambda on the left and the for on the right imply that one already in part views the construct as declarative rather than imperative?

@JeffBezanson
Copy link
Member

The only issue to me is that reversing the order relative to precedent seems more like a spurious difference than correcting an obviously-wrong-in-hindsight historical decision. I'm ok with the python/haskell order.

@kshyatt kshyatt added docs This change adds or pertains to documentation test This change adds or pertains to unit tests labels Jun 27, 2016
@ararslan
Copy link
Member

That's fair. I'd still love to see where for filtering, especially if it follows the for, but I have a feeling that's more of a pipedream. 😜

@JeffBezanson
Copy link
Member

Merged into #16622

@tkelman tkelman deleted the jq/guards branch June 28, 2016 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants