# str(Image) tests too strict about formatting

XMLWordPrintable

## Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
2
• Sprint:
AP F18-6
• Team:

## 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.

## Activity

Hide
John Swinbank added a comment -

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.

Show
John Swinbank added a comment - 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.
Hide
John Swinbank added a comment -

Since Jira hasn't picked it up yet, I'll add that the PR is here: https://github.com/lsst/afw/pull/413

Show
John Swinbank added a comment - Since Jira hasn't picked it up yet, I'll add that the PR is here: https://github.com/lsst/afw/pull/413
Hide
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.

Show
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.
Hide
John Swinbank added a comment -

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.

Show
John Swinbank added a comment - 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.
Hide
John Parejko added a comment -

I think this is a good approach.

Show
John Parejko added a comment - I think this is a good approach.

## People

• Assignee:
John Swinbank
Reporter:
Krzysztof Findeisen
Reviewers:
Krzysztof Findeisen
Watchers:
Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen