-
Notifications
You must be signed in to change notification settings - Fork 60
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
Introduce add!! #226
Introduce add!! #226
Conversation
One |
I don't see this convention documented this way (at least not so succinctly) in the BangBang.jl docs... I'm sure you're right about the convention, but i'd feel better if it were explained in the BangBang docs so we could point to it |
This is nice. Regarding naming: is there a reason we can't call this function |
I would be down with Going for @tkf confirmed for me that this was the correct convention on Slack. |
I am still calling the file |
straying away from this PR... should we have an issue to adopt |
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.
some questions but either answer is fine, so LGTM
@test 16 == add!!(12, @thunk(2*2)) | ||
@test 16 == add!!(16, Zero()) | ||
|
||
@test 16 == add!!(16, DoesNotExist()) # Should this be an error? |
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.
do we have/need an issue for this?
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.
we don't, I am not sure we need one yet.
That particular case should just be a call to |
This closes #113
at least the most important part.
I don't think we need
store!
,and it doesn't do multi-arg accumulation, on the basis that we can add that later, lets not gold-plate this with features we don't yet need.
We can open a follow up issue about that after this is merged
It is named
accumulate!!
after the BangBang.jl convention.a suffix of
!!
means I might mute the input, or i might not, but i will return definately return the thing