Add new --matrix option to multiply commands#526
Add new --matrix option to multiply commands#526cdrini wants to merge 6 commits intoopen-cli-tools:mainfrom
Conversation
ecdae50 to
5af7c99
Compare
5af7c99 to
dfc3c5f
Compare
dfc3c5f to
f656f5d
Compare
gustavohenke
left a comment
There was a problem hiding this comment.
Thanks for taking this up!
I made several suggestions and asked a few questions in this pass.
I'll update the docs to mention matrices as well in a follow-up, unless you feel like doing it while keeping it on brand?
|
@cdrini Wanna take this up again? |
|
Thank you for rebasing, @paescuj ! And thank you for the code review @gustavohenke . Apologies for the delay was a bit stuck on the right approach. Taking another look today. |
- also require M: prefix in placeholder - avoids conflicting with pass-through args - avoids having two ways of specifying the matrix with or without ':' - makes the command easier to read
I confused the term, we're creating a single matrix, as defined by multiple dimensions/axes.
|
Ok! I updated this up with the comments. I decided to tweak the interface slightly to allow/enforce named matrix dimensions, and a I updated the description with the new interface. Would love to know your thoughts! Note: I also fixed a naming error I'd made in the code; there aren't multiple
|
Closes #517
Sample runs:
Single dimension:
Multiple dimensions:
With environment variables: