Arraymancer
Arraymancer copied to clipboard
mratsim
Add support for doing a masked fill from a tensor
This is supported by numpy but was not supported by Arraymancer yet.
I'm hoping I've implemented it in a sufficiently performant way. If I can do it in a better way, please let me know.
It looks fine to me now. One could clarify a bit more in the docstrings how different ranks are handled and specify that the mask and fill value tensors must have the same shape. The next thing 'missing' thing would then be a single element fill I guess (which could be convenient e.g. precisely for the type of NaN test case).
Anyway, I think this fine to merge. Please just drop the Nim 1.4 from the CI. I don't see a purpose in continuing to support it officially. Most of it will work regardless.
It looks fine to me now. One could clarify a bit more in the docstrings how different ranks are handled and specify that the mask and fill value tensors must have the same shape. The next thing 'missing' thing would then be a single element fill I guess (which could be convenient e.g. precisely for the type of NaN test case).
Anyway, I think this fine to merge. Please just drop the Nim 1.4 from the CI. I don't see a purpose in continuing to support it officially. Most of it will work regardless.
How would I drop Nim 1.4 from the CI? Is that something that I can do myself?
It looks fine to me now. One could clarify a bit more in the docstrings how different ranks are handled and specify that the mask and fill value tensors must have the same shape. The next thing 'missing' thing would then be a single element fill I guess (which could be convenient e.g. precisely for the type of NaN test case). Anyway, I think this fine to merge. Please just drop the Nim 1.4 from the CI. I don't see a purpose in continuing to support it officially. Most of it will work regardless.
How would I drop Nim 1.4 from the CI? Is that something that I can do myself?
Ah, sorry. I wanted to add that to my response, but I forgot. Yes, just remove it here in this line:
https://github.com/mratsim/Arraymancer/blob/master/.github/workflows/ci.yml#L17
It looks fine to me now. One could clarify a bit more in the docstrings how different ranks are handled and specify that the mask and fill value tensors must have the same shape. The next thing 'missing' thing would then be a single element fill I guess (which could be convenient e.g. precisely for the type of NaN test case).
I've pushed a new commit that improves the documentation of all the masked_fill procedures (including the ones that fill with a scalar value).
Anyway, I think this fine to merge. Please just drop the Nim 1.4 from the CI. I don't see a purpose in continuing to support it officially. Most of it will work regardless.
Nim 1.4 has been dropped on a separate PR.
Really nice work! Could you update the tutorial vandermonde boolean mask example? I'll merge it after. :partying_face: