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

Length fixes (#100 and a fix for 0.6) #101

Merged
merged 2 commits into from
Apr 7, 2017
Merged

Length fixes (#100 and a fix for 0.6) #101

merged 2 commits into from
Apr 7, 2017

Conversation

iamed2
Copy link
Collaborator

@iamed2 iamed2 commented Apr 5, 2017

#100 Fix

On 0.5 and up, checking for HasLength() || HasShape() is the right way to handle checking whether it is safe to call length. On 0.4 iteratorsize doesn't exist, so applicable(length is our fallback.

0.6 Fix

Since the 265 solution in Julia, generated function methods are only aware of methods that existed when they were compiled. Moving _chain_is to the bottom makes it work for all cases involving iteratorsize in Base and Iterators, but the way forward involves getting rid of generated functions I think.

@codecov-io
Copy link

codecov-io commented Apr 5, 2017

Codecov Report

Merging #101 into master will increase coverage by 0.31%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #101      +/-   ##
==========================================
+ Coverage   94.11%   94.42%   +0.31%     
==========================================
  Files           1        1              
  Lines         374      377       +3     
==========================================
+ Hits          352      356       +4     
+ Misses         22       21       -1
Impacted Files Coverage Δ
src/Iterators.jl 94.42% <100%> (+0.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 023ee1c...8fc901e. Read the comment docs.

@iamed2
Copy link
Collaborator Author

iamed2 commented Apr 5, 2017

Proposal: after this, we make a patch release representing the last release supporting 0.4, then drop support for 0.4 in order to simplify a lot of code and enable us to work on some of the existing ideas in issues/PRs more easily.

src/Iterators.jl Outdated
return true
else
return false
end
Copy link
Member

Choose a reason for hiding this comment

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

return isa(it_size, HasLength) || isa(it_size, HasShape)

?

Copy link
Collaborator Author

@iamed2 iamed2 Apr 5, 2017

Choose a reason for hiding this comment

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

Hah, maybe I should have waited until tomorrow to submit!

I'll change that with a rebase.

@iamed2 iamed2 merged commit 600273b into master Apr 7, 2017
@iamed2 iamed2 deleted the length-fixes branch April 7, 2017 20:40
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.

3 participants