Skip to content
This repository has been archived by the owner on Sep 1, 2020. It is now read-only.

Drop 0.3 and support 0.5 #72

Merged
merged 8 commits into from
Sep 21, 2016
Merged

Drop 0.3 and support 0.5 #72

merged 8 commits into from
Sep 21, 2016

Conversation

iamed2
Copy link
Collaborator

@iamed2 iamed2 commented Sep 20, 2016

🎉

Some Union{}[] and Any[] and Any[...] were changed to [] and [...].

v0.3 support was dropped per the METADATA freeze. Code that only runs on versions before the v0.4.0 release was deleted, and unnecessary Compat usage was deleted. The old test/test.jl file was deleted as its contents are replicated/improved in test/runtests.jl (the test/perf.jl wasn't removed, as it is not yet brought up to date).

v0.5 support was fixed and tested.

@@ -271,7 +275,7 @@ end

@test_zip [1,2,3] [:a, :b, :c] ['x', 'y', 'z']
@test_zip [1,2,3] [:a, :b] ['w', 'x', 'y', 'z']
@test_zip [1,2,3] @compat(Union{})[] ['w', 'x', 'y', 'z']
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The type of the empty array is not material to the test. It was only changed from [] to avoid a deprecation warning from JuliaLang/julia#8578.

Copy link
Contributor

Choose a reason for hiding this comment

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

[] is Union{}[] on 0.3. And this feels like a corner case that still worth testing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright, testing both now.

@@ -122,26 +126,26 @@ end

# Empty arrays
test_imap(
Any[],
Copy link
Contributor

Choose a reason for hiding this comment

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

Also these.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is really not designed to test that. What adding those would really be testing is Union{}[] == Any[] which is not in the scope of Iterators.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, those test was written in that way to begin with.

It's hard to say if it's in scope of Iterators but it doesn't hurt to not change the tests.

@@ -126,26 +126,26 @@ end

# Empty arrays
test_imap(
[],
Copy link
Contributor

Choose a reason for hiding this comment

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

This and the last one is still wrong.

@@ -294,7 +299,7 @@ macro test_enumerate(input)
end

@test_enumerate [:a, :b, :c]
@test_enumerate @compat(Union{})[]
@test_enumerate []
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, here..... I was trying to not leave a comment at every single place.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ugh, sorry. Missed that one in the first batch.

@@ -299,7 +309,8 @@ macro test_enumerate(input)
end

@test_enumerate [:a, :b, :c]
@test_enumerate []
@test_enumerate Union{}[]
@test_enumerate Any[]
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that on 0.4+ Any[] is the same with []. Doesn't really mater which one to use.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep. Just being more explicit as these tests are no longer just meant to test "empty arrays".

@iamed2 iamed2 merged commit b176ee0 into master Sep 21, 2016
@yuyichao yuyichao deleted the point-five branch September 21, 2016 17:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants