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

allow x[i,j;k] syntax to be parsed #8250

Merged
merged 1 commit into from
Sep 30, 2014

Conversation

jakebolewski
Copy link
Member

@jakebolewski
Copy link
Member Author

It's nice that after #8190 this also prints correctly.

@JeffBezanson
Copy link
Member

Thanks for taking care of this. This is pretty much right, but I found this:

> (julia-parse "[b,c;d,e]")
(vcat (parameters d e) b c)

julia> [1,2;3,4]
ERROR: syntax: invalid keyword argument syntax "3" (expected assignment)

julia> [1, 2; a=3]
ERROR: syntax: invalid keyword argument syntax "a=3" (expected assignment)

@jakebolewski
Copy link
Member Author

I thought that this syntax was supposed to be parseable but still not valid Julia syntax (so that the JUMP guys could use it in their macros, but it remains an invalid Julia expression)?

Obviously it would be better to agree on what this would mean and make it valid syntax as well. I don't see how [1,2; a=3] could ever be a valid expression.

@JeffBezanson
Copy link
Member

Parsing a[i,j;k] is ok, but [1,2;3,4] is sketchier since it looks like matrix syntax. Also the error message in the last case is pretty confusing.

@jakebolewski
Copy link
Member Author

I agree it is sketchy, what would you like to do? Throw a better error message? (Something like, Error: syntax: [1,2;3,4] is not valid matrix syntax)?

@JeffBezanson
Copy link
Member

Yeah that would be better, until we figure something else out.

@jakebolewski
Copy link
Member Author

We currently give this error for cell arrays:

julia> {1,2;3,4}
ERROR: unsupported or misplaced expression parameters

would it be sufficient to make the error messages the same (for 0.3)? We could always come back and clean up this up later for 0.4. As JUMP will probably be using 0.3 for a while, it would be nice to backport this change for them.

@joehuchette
Copy link
Member

Backporting this to 0.3 would be fantastic for us, if that's a reasonable request. Thanks for this, @jakebolewski!

@IainNZ
Copy link
Member

IainNZ commented Sep 7, 2014

💯

@jakebolewski
Copy link
Member Author

Updated now that the 0.3.1 window is closing. This should preserve the old syntax error behavior now.

julia> [1,2;3,4]
ERROR: syntax: unexpected semicolon in array expression

julia> {1,2;3,4}
ERROR: syntax: unexpected semicolon in array expression

julia> Int[1,2;3,4]
ERROR: syntax: unexpected semicolon in array expression

julia> [1,2;a=3]
ERROR: syntax: unexpected semicolon in array expression

julia> {1,2;a=3}
ERROR: syntax: unexpected semicolon in array expression

julia> Int[1,2;a=3]
ERROR: syntax: unexpected semicolon in array expression

@jakebolewski jakebolewski force-pushed the jcb/newarraysyntax branch 2 times, most recently from c3df3dc to f8e66b8 Compare September 18, 2014 04:49
@IainNZ
Copy link
Member

IainNZ commented Sep 18, 2014

👍

(receive
(kws args) (separate kwarg? (cdddr e))
(lower-kw-call f (append kws (cdr (caddr e))) args))))
(if (eq? f 'vcat)
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't work, because it gives

julia> vcat(x;a=1)
ERROR: syntax: unexpected semicolon in array expression

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 don't know why I didn't add the check when expanding 'vcat, will fix.

@jakebolewski
Copy link
Member Author

@JeffBezanson can you have another look at this so it can be backported?

JeffBezanson added a commit that referenced this pull request Sep 30, 2014
@JeffBezanson JeffBezanson merged commit beb5486 into JuliaLang:master Sep 30, 2014
@mlubin
Copy link
Member

mlubin commented Sep 30, 2014

👍

@jakebolewski
Copy link
Member Author

@JuliaBackports

@joehuchette
Copy link
Member

Awesome, thanks @jakebolewski!

@andreasnoack
Copy link
Member

Is this one causing the ErrorException("new: too many arguments")s I get when I try to build latest master? E.g. at LoadError(at "bitarray.jl" line 379, and LoadError(at "osutils.jl" line 38

@jakebolewski
Copy link
Member Author

Was it this or 7f2cf32?

@andreasnoack
Copy link
Member

I did a git pull and got many weird errors. However, a git reset --hard origin/master seems to have solved the problem.

@ivarne
Copy link
Member

ivarne commented Oct 3, 2014

Was there any problem with this, or is it time to backport to release-0.3?

I've used my quota for mistakes this week, so someone else will have to do the honours 😜

@staticfloat
Copy link
Member

895639a :)

@ivarne ivarne added the breaking This change will break code label Oct 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants