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

@system: test macro calls without parentheses #156

Merged
merged 3 commits into from
Feb 6, 2020
Merged

Conversation

ueliwechsler
Copy link
Collaborator

Complete an element of checklist in #133.

See here , the solution is trivial 🙈

The syntax needs to be

@system x' = Ax + Bu  xX uU

without the commas.

I added the corresponding tests

@mforets mforets mentioned this pull request Feb 1, 2020
12 tasks
@schillic
Copy link
Member

schillic commented Feb 2, 2020

I do not like this syntax.

@ueliwechsler
Copy link
Collaborator Author

I do not like this syntax.

Leaving out the commas?

@schillic
Copy link
Member

schillic commented Feb 2, 2020

Yes, this is not how you write a system definition on paper. I am fine with having this as an option of course. It works already, we can actually not avoid it, and I guess there is no way that , works without using (·) because this is how Julia parses expressions.
But I suggest to use the syntax @system(x' = Ax + Bu, x∈X, u∈U) rather than @system x' = Ax + Bu x∈X u∈U when showcasing.

@blegat
Copy link
Member

blegat commented Feb 2, 2020

I agree with @schillic, this is consistent with
http://www.juliaopt.org/JuMP.jl/v0.20.0/style/#JuMP-macro-syntax-1

@mforets
Copy link
Member

mforets commented Feb 6, 2020

See here , the solution is trivial see_no_evil

It is fun that we didn't realize this earlier :)

this is consistent with http://www.juliaopt.org/JuMP.jl/v0.20.0/style/#JuMP-macro-syntax-1

I agree as well that in @system the commas make it easier to parse the expression (for humans). On the other hand this behavior is already available, so i think it is a good idea to add the test proposed in this PR.

test/@system.jl Outdated Show resolved Hide resolved
test/@system.jl Outdated Show resolved Hide resolved
mforets and others added 2 commits February 6, 2020 09:28
Co-Authored-By: Christian Schilling <schillic@informatik.uni-freiburg.de>
Co-Authored-By: Christian Schilling <schillic@informatik.uni-freiburg.de>
@mforets mforets merged commit 658d2f2 into master Feb 6, 2020
@mforets mforets deleted the ueliwechsler/133c branch February 6, 2020 12:29
@mforets mforets changed the title @system: make macro calls work without parentheses @system: test macro calls without parentheses Feb 6, 2020
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.

None yet

4 participants