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

support for generators instead of curly syntax in Julia 0.5 #807

Merged
merged 2 commits into from
Jul 23, 2016

Conversation

mlubin
Copy link
Member

@mlubin mlubin commented Jul 18, 2016

Now that Julia master has support for generator syntax with filters (JuliaLang/julia#550), we can now support this:

@constriant(m, sum(x[i] for i in 1:N if i != 2) <= 1)

This PR enables support for generator syntax wherever we used to use curly braces. Once we drop support for Julia 0.4, we should also deprecate the curly syntax.

@codecov-io
Copy link

codecov-io commented Jul 18, 2016

Current coverage is 84.68% (diff: 4.13%)

Merging #807 into master will decrease coverage by 2.82%

@@             master       #807   diff @@
==========================================
  Files            18         18          
  Lines          4075       4219   +144   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           3566       3573     +7   
- Misses          509        646   +137   
  Partials          0          0          

Powered by Codecov. Last update 4a8d878...333c31b

@mlubin
Copy link
Member Author

mlubin commented Jul 20, 2016

Everything's passing. Will merge in a few days if there are no objections, followed by a post to julia-opt calling for testers for the new syntax.

@joehuchette
Copy link
Contributor

We're not losing any functionality here, correct? If so, I'm all for it.

@mlubin
Copy link
Member Author

mlubin commented Jul 20, 2016

No loss of functionality, if this syntax had been around 3 years ago we would have definitely chosen it over the curly braces.

end
end
for level in length(x.args[2].args):-1:2
_idxvar, idxset = parseIdxSet(x.args[2].args[level]::Expr)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the type assert needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Dunno, just following the previous code here

@joehuchette
Copy link
Contributor

What about deprecating the curly bracket syntax?

@mlubin
Copy link
Member Author

mlubin commented Jul 20, 2016

What about deprecating the curly bracket syntax?

I was thinking about holding off on that until 0.15, if 0.14 is the last release that supports 0.4. People who want to be able to use the same JuMP models on Julia 0.4 and 0.5 without annoying deprecation warnings can then use JuMP 0.14.

@mlubin
Copy link
Member Author

mlubin commented Jul 20, 2016

This is also a more invasive change than the big renaming, since it requires more than find and replace. Hopefully @tkelman won't tell us that we'll be driving cars off the road because of it.

@IainNZ
Copy link
Collaborator

IainNZ commented Jul 21, 2016

Its going to be nice to not have to explain the { anymore!

@mlubin mlubin merged commit f1eff33 into master Jul 23, 2016
@mlubin mlubin deleted the ml/generators branch July 23, 2016 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants