-
Notifications
You must be signed in to change notification settings - Fork 5
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
Implement splatting variables #255
Conversation
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.
It looks fine, I thought of a different implementation to be honest, but this seems to work as well, so it shouldn't be a big problem I guess. I also added a couple of extra tests, please double check if those make sense.
P.S. Regarding the implementation I had in mind is to have Splat
as an index
in the proxylabel
, not as a wrapper for proxied
. Later on we could dispatch on the type of the index when creating the node, but your implementation seems to work fine too.
(I guess docs build may fail because the new struct is not present in the documentation as a |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #255 +/- ##
==========================================
- Coverage 90.86% 90.61% -0.25%
==========================================
Files 15 15
Lines 2090 2121 +31
==========================================
+ Hits 1899 1922 +23
- Misses 191 199 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@wouterwln the |
The implementation is slightly ugly since I'm creating a vector of
ProxyLabel
s which I am splatting again, but I think this is the issue with splatting altogether. At least this implementation works and is supposed to fix the issue at hand. Also @bvdmitri do you have an alternative for my implementation ofBase.iterate
forVariableRef
s? I couldn't get it to work without duplicating all code unfortunately.