Ensure broadcasted functions retain their bounds checks#64
Ensure broadcasted functions retain their bounds checks#64andyferris merged 2 commits intomasterfrom
Conversation
andyferris
left a comment
There was a problem hiding this comment.
Haha awesome - thanks for finding this one.
(Note: the original intent for _f(d) was to be an alias for d.f in the case that I overload getproperty on AbstractDict (as a shortcut for dictionaries with Symbol keys) so perhaps it could be renamed?)
The inbounds assertion within the map! implementation, combined with propagate_inbounds on gettokenvalue(d::BroadcastedDictionary, t) leads to removing the bound check on any user-defined function which is broadcasted. This adds an extra layer of dispatch to protect the user-defined function which is broadcasted.
5dbd4ca to
477e6f9
Compare
Yeah I figured, but I was lazy 😆. I renamed it to Also fixed the tests by making sure |
|
Great! 👍 There seems to be an UndefVarError in the CI. |
This was weird. I think it may have been something to do with Anyway, it turns out that those tests always passed regardless because CI runs with |
Codecov Report
@@ Coverage Diff @@
## master #64 +/- ##
==========================================
- Coverage 74.06% 74.02% -0.05%
==========================================
Files 19 19
Lines 1720 1721 +1
==========================================
Hits 1274 1274
- Misses 446 447 +1
Continue to review full report at Codecov.
|
Duh! 🤦♂️ Nice solution. |
I stole it from Base 😆 i think it might be nicer if the whole set of tests ran in CI both with and without |
The inbounds assertion within the map! implementation, combined with
propagate_inbounds on gettokenvalue(d::BroadcastedDictionary, t) leads
to removing the bound check on any user-defined function which is
broadcasted. This adds an extra layer of dispatch to protect the
user-defined function which is broadcasted.
Fixes #63