GeezerVison

coverage: 98.838% (-0.9%) from 99.743% when pulling 9e51b87 on eriknw:wip_setdiag into bf28a79 on python-graphblas:main.

This is ready for review. I updated it to implement option 2 from #487 (comment) by adding mask= and accum= arguments to setdiag.

The main remaining question I have is regarding clear_missing=True argument. Does it make sense? Is the name good? Or should it not be included at all? Also, are the code and docstrings clear enough, or should any more docs be added?

I think the pros and cons from #487 (comment) are still accurate and I think are worth rereading when reviewing this PR. I am +1 for adding matrix.setdiag for reasons stated in that comment.

CC @q-aaronzolnailucas @SultanOrazbayev @jim22k

Do you plan to allow A(mask, accum=accum).setdiag(v)? Which would just delegate to the keywords in the setdiag method.

Unsure. I removed this. It would be very easy to add back. What do you think?

Do you plan to allow A(mask, accum=accum).setdiag(v)? Which would just delegate to the keywords in the setdiag method.

Unsure. I removed this. It would be very easy to add back. What do you think?

Add it as an issue to discuss, but don't hold up this PR for that.

For a 0-length vector where k == 0, I could maybe see an exception. But if k != 0 and the off-diagonal is length zero, that is out of range. I think raising is the correct approach.

This (allowing to set 0-length when k == 0) is actually what setdiag in scipy.sparse does. I think this is a good idea and let's us better handle length 0 dimensions. For example, should it be possible to set any diagonal in a 0x2 Matrix? My latest commit allows this.

For the record, scipy.sparse setdiag also allows the size of the vector of new values to be too large or too small. Should we follow suite, or require that the vector exactly match the size of the diagonal, which is what we currently do?