-
Notifications
You must be signed in to change notification settings - Fork 9
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
WIP Remove dependency on inference #7
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7 +/- ##
==========================================
+ Coverage 81.69% 82.51% +0.82%
==========================================
Files 18 18
Lines 508 549 +41
==========================================
+ Hits 415 453 +38
- Misses 93 96 +3
Continue to review full report at Codecov.
|
|
||
function _map(source::Enumerable, f::Function, f_expr::Expr, ::Base.HasEltype) | ||
TS = eltype(source) | ||
T = Base._return_type(f, Tuple{TS,}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The strategy here is to use the information from inference if it gives us a concrete type, otherwise not use that information. I think that is the same strategy that Broadcast is using now? As far as I understand it, that would be an ok use of inference.
The question though is whether it actually buys us anything, or whether we could just never use it. At least for a intermediate period it would help with moving things over: it will take a while to change all the sinks to accept iterators that have the EltypeUnknown()
trait, and until that is done this implementation here would still return things with HasEltype()
in all cases where it does so today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The strategy here is to use the information from inference if it gives us a concrete type, otherwise not use that information. I think that is the same strategy that Broadcast is using now? As far as I understand it, that would be an ok use of inference.
FWIW, I don't think the question is whether the inferred type is concrete or not. It's rather that the user-visible behavior should be the same whether or not inference gives you a precise type or Any
.
No description provided.