-
-
Notifications
You must be signed in to change notification settings - Fork 398
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
Pass names (index_vars) to container extensions. #3088
Pass names (index_vars) to container extensions. #3088
Conversation
Codecov ReportBase: 97.63% // Head: 97.64% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3088 +/- ##
==========================================
+ Coverage 97.63% 97.64% +0.01%
==========================================
Files 32 32
Lines 4305 4330 +25
==========================================
+ Hits 4203 4228 +25
Misses 102 102
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
One thought I had: it might make sense to make names
the last argument? Then we'd have a 4, 3, 2 argument methods, each dropping the last argument.
Yeah, I guess so. My reasoning was that names go naturally together with the indices they refer to, but there is a certain appeal to dropping (or adding) the last argument and keep the first arguments the same. No strong opinions from my side, can update if you prefer. |
Yeah, I think please swap. Let's have consistency in the dropping argument order. |
Our container in
SparseVariables
supports storing the names of the subindices (index_vars
), but these are not currently passed to containers.See sintefore/SparseVariables.jl#19