enthought / chaco

Chaco is a Python package for building interactive and custom 2-D plots.
http://docs.enthought.com/chaco/
Other
292 stars 99 forks source link

FIX: suppress invalid data warnings for handling NaNs in DataRange classes #835

Closed nicolasap-dm closed 1 year ago

nicolasap-dm commented 1 year ago

Fixes #834

The default behavior of float comparison with nans is consistent with the mask_data abstract specification (being "inside the range" implies being finite), so it should be fine to confidently use it without warnings.

By the way, all uses of mask_data in the Chaco codebase outside of the classes themselves are already paired with a & nan_mask. I'm not sure if that's redundant, but it makes me confident that exluding nans from masks is expected everywhere.


¹ ~a curious finding: apparently transposing an array defuses the warning in the first place~

```python >>> import numpy as np >>> nan = float("nan") >>> np.array([1.0, nan]) > 0.5 __main__:1: RuntimeWarning: invalid value encountered in greater array([ True, False]) >>> np.array([[1.0], [nan]]).transpose()[0] > 0.5 array([ True, False]) ``` Edit: I was wrong, there is an issue of state. Whichever of the above comparisons gets run first will emit the warning.
nicolasap-dm commented 1 year ago

The tests check that NaNs are False in masks.

They try to check also that warnings are not emitted, however it is not guaranteed that they would spot a regression, because if a matching warning has already been emitted anywhere else in the test suite, the "once" rule on warnings will already have prevented it to be emitted again.

From the docs, it seems like it should be overcomable (emphasis mine):

One thing to be aware of is that if a warning has already been raised because of a once/default rule, then no matter what filters are set the warning will not be seen again unless the warnings registry related to the warning has been cleared.

https://docs.python.org/3/library/warnings.html#testing-warnings

however, clearing the registry doesn't seem to be easy or supported: https://bugs.python.org/issue21724.

CC @mdickinson since this came up in offline discussion.


Mark, you're welcome to review if you have time. The core of the PR is based on @rkern's suggestion on Slack.