Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (897a7cb) 89.24% compared to head (ab0a20f) 89.26%.

@@            Coverage Diff             @@
##             main    #5084      +/-   ##
==========================================
+ Coverage   89.24%   89.26%   +0.02%     
==========================================
  Files          88       88              
  Lines       22191    22233      +42     
  Branches     4855     4863       +8     
==========================================
+ Hits        19805    19847      +42     
  Misses       1641     1641              
  Partials      745      745              

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Thanks for reviewing @lbdreyer! I think I addressed all your comments, let me know if you need anything else. 👍

About the line in the __array_finalize__ method of Weights that is not covered by tests:

I copy-pasted this from https://numpy.org/doc/stable/user/basics.subclassing.html#slightly-more-realistic-example-attribute-added-to-existing-array. However, if I understand the description of the __array_finalize__ in the docstring here correctly, then obj can only be None if the __new__ method of the child class calls the __new__ method of the parent class (here: np.ndarray) directly. Since the __new__ method of the child class does not do that but only uses np.asarray(...).view(cls), I think obj can never be None here, so we could probably safely remove this line. My only question would then be why the example uses this line? Maybe I overlooked something?

The linkcheck reports that the Twitter link is broken

 community/index: line   28) broken    https://twitter.com/scitools_iris - 403 Client Error: Forbidden for url: https://twitter.com/scitools_iris

For me that link works. Not sure if this is related to this PR.

I just re-ran the link check and it’s OK now.

About the line in the array_finalize method of Weights that is not covered by tests:

I copy-pasted this from https://numpy.org/doc/stable/user/basics.subclassing.html#slightly-more-realistic-example-attribute-added-to-existing-array. However, if I understand the description of the array_finalize in the docstring here correctly, then obj can only be None if the new method of the child class calls the new method of the parent class (here: np.ndarray) directly. Since the new method of the child class does not do that but only uses np.asarray(...).view(cls), I think obj can never be None here, so we could probably safely remove this line. My only question would then be why the example uses this line? Maybe I overlooked something?

I agree. The numpy docs imply that of the three ways that __array_finalize__ can be called,

1. explicit constructor _Weights(),
2. view casting np.zeros((3,)).view(_Weights),
3. new from template existing_weights_object[:1]

Only the explicit constructor may have obj as None. But as you have pointed out obj is always returned by the __new__ method as np.asarray(...).view(cls),. It is probably only relevant if you were to call super().__new__()
So I'd be happy were you to remove if obj is None: return