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

Fix compatibility issues with v0.4 ntuple #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hackjackhack
Copy link

Added a version checking before calling ntuple(). First time sending a PR.

@timholy
Copy link
Contributor

timholy commented Jun 8, 2015

Thanks for tackling this!

Note that even on julia 0.3, this is valid syntax:

               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: http://docs.julialang.org
   _ _   _| |_  __ _   |  Type "help()" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.3.7-pre+1 (2015-02-17 22:12 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit d15f183* (110 days old release-0.3)
|__/                   |  x86_64-linux-gnu

julia> ntuple(sin, 3)
(0.8414709848078965,0.9092974268256817,0.1411200080598672)

So I don't think you even need the version check.

@stevengj
Copy link

stevengj commented Jun 8, 2015

But you should change the REQUIRE file to julia 0.3.

@carlobaldassi
Copy link
Contributor

But you should change the REQUIRE file to julia 0.3.

that syntax works in 0.2 too:

julia> VERSION
v"0.2.1+22"

julia> ntuple(identity, 4)
(1,2,3,4)

I say this would only need the commits to be squashed.

@hackjackhack
Copy link
Author

Commits squashed.

@carlobaldassi
Copy link
Contributor

Squashed too much? It seems the commit has been merged with the previous one from master, and I come out as the author instead of you.

@hackjackhack
Copy link
Author

Oops! Fortunately, the last commit happens to be "Fix compatibility issues with julia v0.4"
I don't really care about the author stuff. Just want to get rid of the annoying warning message.

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.

4 participants