Codecov Report

Merging #153 (ca1dc16) into main (6b2ffb0) will decrease coverage by 0.45%.
The diff coverage is 50.00%.

❗ Current head ca1dc16 differs from pull request most recent head 58434f7. Consider uploading reports for the commit 58434f7 to get more accurate results

@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
- Coverage   75.77%   75.32%   -0.46%     
==========================================
  Files           9        9              
  Lines         706      697       -9     
==========================================
- Hits          535      525      -10     
- Misses        171      172       +1     

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Hmm. Maybe #151 isn't the correct solution after all.

Hi @pllim, after some local testing, I think the best solution would be to drop masked_where() entirely. In the place that causes the error you found, all we really need is the mask; the other attributes that come with a masked_array are superfluous. All they do to create the mask is call isfinite(), so we can just do the same here.

This usage of masked_where() appears a few places in specreduce and I believe they should all should be changed in the same effort. That could be this one, or if you like, I can try it in a different pull request.

If you want to change mask logic, it is up to you as I am not familiar with the algorithms here. I just did track down why numpy dev is giving error and it might or might not be upstream bug. I am going to ask numpy people (see numpy/numpy#22720).

As for the coverage, it will fix itself when numpy is released and your coverage job picks it up. Obviously it is covered because tests fail with dev numpy.

Maybe we can merge by the end of the week if numpy makes it clear that they have no intention of backing out this behavior.

@pllim I opened #155 to make the edits I talked about earlier since most of the places that trigger this issue don't actually need to use mask_invalid(). Do you have any objections to that approach?

The other PR should probably still integrate the changes you made to tox.ini in order to catch these errors more quickly in the future.

@ojustino , yes, please do cherry-pick the dev numpy job over. If #155 supersedes this PR, feel free to close this without merge. Thanks!