Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-16291

str(Image) tests too strict about formatting

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw
    • Labels:

      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

            Activity

            No builds found.
            krzys Krzysztof Findeisen created issue -
            krzys Krzysztof Findeisen made changes -
            Field Original Value New Value
            Epic Link DM-14447 [ 80385 ]
            krzys Krzysztof Findeisen made changes -
            Link This issue relates to DM-15682 [ DM-15682 ]
            krzys Krzysztof Findeisen made changes -
            Priority Major [ 3 ] Critical [ 2 ]
            Parejkoj John Parejko made changes -
            Sprint AP F18-6 [ 751 ]
            Parejkoj John Parejko made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            Parejkoj John Parejko made changes -
            Status In Progress [ 3 ] To Do [ 10001 ]
            Parejkoj John Parejko made changes -
            Watchers John Parejko, Krzysztof Findeisen [ John Parejko, Krzysztof Findeisen ] Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen [ Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen ]
            Parejkoj John Parejko made changes -
            Assignee John Parejko [ parejkoj ]
            swinbank John Swinbank made changes -
            Assignee John Swinbank [ swinbank ]
            Hide
            swinbank 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
            swinbank 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.
            swinbank John Swinbank made changes -
            Reviewers Krzysztof Findeisen [ krzys ]
            Status To Do [ 10001 ] In Review [ 10004 ]
            Hide
            swinbank 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
            swinbank 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
            krzys 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
            krzys 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.
            krzys Krzysztof Findeisen made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            Hide
            swinbank 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
            swinbank 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.
            swinbank John Swinbank made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]
            Hide
            Parejkoj John Parejko added a comment -

            I think this is a good approach.

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

              People

              Assignee:
              swinbank John Swinbank
              Reporter:
              krzys Krzysztof Findeisen
              Reviewers:
              Krzysztof Findeisen
              Watchers:
              Jim Bosch, John Parejko, John Swinbank, Krzysztof Findeisen
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.