Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: afw
-
Labels:
-
Story Points:2
-
Epic Link:
-
Sprint:AP F18-6
-
Team:Alert Production
Description
While the changes introduced in DM-15682 passed formal testing, I discovered that they do not pass if afw is built against NumPy 1.13 – that version of numpy includes more whitespace in the array than the test expects:
AssertionError: '[[0. 0.]\n [0. 0.]]' not found in '[[ 0. 0.]\n [ 0. 0.]], bbox=(minimum=(0, 0), maximum=(1, 1))'
|
While the build failure can be worked around by upgrading NumPy to 1.14 or later (which will soon be required anyway), the consensus on #dm-build-problems was that any future formatting changes in NumPy would break the test again.
Please modify the tests so that they're less sensitive to NumPy's formatting decisions, preferably while keeping some diagnostic of whether the array is included in the string.
Attachments
Issue Links
- relates to
-
DM-15682 Add str() for afw::Image and afw::Mask
- Done
Activity
Since Jira hasn't picked it up yet, I'll add that the PR is here: https://github.com/lsst/afw/pull/413
I disagree. Delegating to NumPy is an implementation detail; what we really should be testing is "does the string contain a reasonable array-like representation of the image pixels?".
That said, the original version of the test also assumed a NumPy implementation, if less explicitly, and this approach does solve the forward-compatibility problem.
Krzysztof Findeisen
added a comment - - edited I disagree. Delegating to NumPy is an implementation detail; what we really should be testing is "does the string contain a reasonable array-like representation of the image pixels?".
That said, the original version of the test also assumed a NumPy implementation, if less explicitly, and this approach does solve the forward-compatibility problem. Thanks for the review!
I agree that one could imagine a test for “a reasonable representation”, but I think then we'd need to bring the implementation in-house: delegating generation of output to NumPy while testing against our own internal standard seems doomed to hopeless fragility.
Anyway, this will do for this ticket; merged and done.
I think this is a good approach.
The problem here arises because these tests conflate two questions: “are we correctly delegating to NumPy stringification?” and “does NumPy stringification look like we expect it to?”. I suggest that the relevant question is really the former.
Krzysztof Findeisen, would you mind taking a look at my changes? I hope this will work on NumPy 1.13 but I don't have a convenient system to check on.