nwb-schema icon indicating copy to clipboard operation
nwb-schema copied to clipboard

TimeSeries.resolution should be NaN if not known instead of -1.0

Open rly opened this issue 5 years ago • 4 comments

In the best practices, we encourage the use of NaN to represent missing data. But for TimeSeries.data.resolution, the docstring says to use -1.0 if the value is unknown. We should use NaN here as well.

rly avatar Nov 22 '19 18:11 rly

@ajtritt There were some problems with the testing framework not understanding that Nan == NaN should be true when in cases where you compare the desired with the actual value in a test.

t-b avatar Nov 22 '19 18:11 t-b

In Python: np.testing.assert_array_equal correctly tests whether nan == nan. We would have to replace all self.assertEqual calls for numeric values in PyNWB with np.testing.assert_array_equal.

Or we could subclass unittest.TestCase, change the superclass of all tests to this new class, and override the assertEqual method in the new class to handle nan values.

def assertEqual(self, first, second, msg=None)
    if np.isnan(first) and np.isnan(second)
        return True
    else:
        return super().assertEqual(first, second, msg)

Not sure what would have to be done on the Matlab side.

rly avatar Nov 23 '19 01:11 rly

I like the idea of using numpy testing. There is a solution in MATLAB. The naming isn't exactly elegant, but it should work:

https://www.mathworks.com/help/matlab/ref/isequalwithequalnans.html

On Fri, Nov 22, 2019 at 5:56 PM Ryan Ly [email protected] wrote:

In Python: np.testing.assert_array_equal correctly tests whether nan == nan. We would have to replace all self.assertEqual calls for numeric values in PyNWB with np.testing.assert_array_equal.

Or we could subclass unittest.TestCase, change the superclass of all tests to this new class, and override the assertEqual method in the new class to handle nan values.

def assertEqual(self, first, second, msg=None) if np.isnan(first) and np.isnan(second) return True else: return super().assertEqual(first, second, msg)

Not sure what would have to be done on the Matlab side.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/NeurodataWithoutBorders/nwb-schema/issues/335?email_source=notifications&email_token=AAGOEEW65ZQ5UJCHRMCMWILQVCEWNA5CNFSM4JQUM5MKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE7KRKI#issuecomment-557754537, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGOEEXZFSS5EIUJTJ2VSRTQVCEWNANCNFSM4JQUM5MA .

-- Ben Dichter, PhD Data Science Consultant

bendichter avatar Nov 23 '19 02:11 bendichter

Actually, this looks like it'll work too: https://www.mathworks.com/help/matlab/ref/isequaln.html

On Fri, Nov 22, 2019 at 6:08 PM Ben Dichter [email protected] wrote:

I like the idea of using numpy testing. There is a solution in MATLAB. The naming isn't exactly elegant, but it should work:

https://www.mathworks.com/help/matlab/ref/isequalwithequalnans.html

On Fri, Nov 22, 2019 at 5:56 PM Ryan Ly [email protected] wrote:

In Python: np.testing.assert_array_equal correctly tests whether nan == nan. We would have to replace all self.assertEqual calls for numeric values in PyNWB with np.testing.assert_array_equal.

Or we could subclass unittest.TestCase, change the superclass of all tests to this new class, and override the assertEqual method in the new class to handle nan values.

def assertEqual(self, first, second, msg=None) if np.isnan(first) and np.isnan(second) return True else: return super().assertEqual(first, second, msg)

Not sure what would have to be done on the Matlab side.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/NeurodataWithoutBorders/nwb-schema/issues/335?email_source=notifications&email_token=AAGOEEW65ZQ5UJCHRMCMWILQVCEWNA5CNFSM4JQUM5MKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE7KRKI#issuecomment-557754537, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGOEEXZFSS5EIUJTJ2VSRTQVCEWNANCNFSM4JQUM5MA .

-- Ben Dichter, PhD Data Science Consultant

-- Ben Dichter, PhD Data Science Consultant

bendichter avatar Nov 23 '19 02:11 bendichter