-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add real world test suite #27
Conversation
In the process of reviewing this, I realised we needed some really clean test data files for us and those responsible for the storage systems themselves, hence the last commit. Will review the rest of it properly tomorrow. |
@valeriupredoi Why do we get warnings from the
Surely the test should fail when it can't open the file, before we get to that point? Or is the data empty anyway and it's failing first? |
@bnlawrence those are warnings that don't trigger a test fail unless I setup pytest to fail on warning; I don't think there is an actual issue coming from them to be honest, I think it's just that the data is fully masked in that particular slice - note that those warnings come from the pure-Numpy block ncfile = str(test_data_path / "emac.nc")
active = Active(ncfile, "aps_ave")
active._version = 0
d = active[4:5, 1:2]
mean_result = np.mean(d) that we can simply remove since the scope of the test is to show the fail on a Classic or netCDF3 file - what you reckon? |
Very good point, cheers! |
I figured the easiest way to handle this was in the test, rather than remove, although as you say the scope of the test doesn't cover it., but there is a bigger problem to be handled in active itself, which needs to handle this without errors. I've dealt with it in ee26bd
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy that this can be merged now, the outstanding bits are in other tickets.
# as it happens it is is possible for a slice to be | ||
# all missing, so for the purpose of this test we | ||
# ignore it, but the general case should not. | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ouch! Haha, this is a proper hack, but heyho - this bit of the test is not important at all, I'll have a proper fix in the future, cheers for it for now @bnlawrence
Hey guys @bnlawrence @davidhassell here's a pretty serious test module that looks at:
h5py
_FillValue
andmissing_value
tests/test_data
Active()
does silly well in all these cases, it's brutal how good it performs! 🍺