Address some docs build warnings #157
Conversation
| @@ -169,7 +169,7 @@ class Diagonal(LinearOperator): | |||
|
|
|||
| def __init__( | |||
| self, | |||
| diagonal: Union[Array, BlockArray], | |||
There was a problem hiding this comment.
@Michael-T-McCann: I think this was originally added by you. Please conform that the change from Array to JaxArray (which silences the warning) is indeed correct.
There was a problem hiding this comment.
I think in this case, changing Array to JaxArray means that the developer using the linop has to create the jax array beforehand. If we leave it as Array, it can take a numpy array or anything and convert it inside it using the ensure_on_device method.
Essentially, the warning means that there is a conversion to JaxArray from np.array and I'm not sure if we want to allow the linop to be created using anything other than device/block arrays. To my knowledge, it doesn't affect anything code-wise due to the ensure_on_device but it brings attention to if we want to restrict it to that type.
There was a problem hiding this comment.
That's a reasonable argument, but it seems that all the other LinOps use JaxArray rather than Array. In fact, although Array is indeed defined in scico.typing, from a quick scan I couldn't find a single instance of it actually being used.
There was a problem hiding this comment.
Given that other code uses JaxArray, let's go with JaxArray here.
As a general principle, I believe we are not going out of our way to be interoperable with numpy.
Codecov Report
@@ Coverage Diff @@
## main #157 +/- ##
=======================================
Coverage 92.13% 92.13%
=======================================
Files 48 48
Lines 3294 3294
=======================================
Hits 3035 3035
Misses 259 259
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Address some docs build warnings.