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

Add str() for afw::Image and afw::Mask

    XMLWordPrintable

Details

    • 1

    Description

      It would be a useful convenience to not have to always append .getArray() when wanting to print an afw::image when debugging. It should be simple to add a std::cout/str() method that delegates to the existing .getArray() and adds the xy0/origin, and other relevant (short) metadata (not the maskPlaneDict for Masks though).

      Attachments

        Issue Links

          Activity

            price Paul Price added a comment -

            Firstly, .getArray() should be spelled .array these days.
            Secondly, if this has to be done, please make it short (a single line or two) — unlike the table printing, which can be thousands of lines long.

            price Paul Price added a comment - Firstly, .getArray() should be spelled .array these days. Secondly, if this has to be done, please make it short (a single line or two) — unlike the table printing, which can be thousands of lines long.
            tjenness Tim Jenness added a comment -

            At the very least I'd love it if str() gave the dimensions and origin information. I agree that you don't want a 4Kx4K dump but on the other hand, doesn't numpy have a parameter for controlling how much content is reported for str() so shouldn't we respect that setting?

            tjenness Tim Jenness added a comment - At the very least I'd love it if str() gave the dimensions and origin information. I agree that you don't want a 4Kx4K dump but on the other hand, doesn't numpy have a parameter for controlling how much content is reported for str() so shouldn't we respect that setting?
            Parejkoj John Parejko added a comment -

            That was my point of delegating to .getArray(): just print whatever that does, plus dimensions and origin.

            Parejkoj John Parejko added a comment - That was my point of delegating to .getArray() : just print whatever that does, plus dimensions and origin.
            Parejkoj John Parejko added a comment -

            tjenness, price: Since you both expressed opinions about this, I figured you'd want to review it. It delegates the array collapsing to numpy, and includes the bbox. I think that means it also tells you the xy0 (as the minimum of the bbox): we weren't able to come up with a test case that behaved otherwise, but maybe you know of one?

            Here's what an example str(maskedImage) looks like.

            >>> print(lsst.afw.image.MaskedImage(100,100))
            image=[[0. 0. 0. ... 0. 0. 0.]
             [0. 0. 0. ... 0. 0. 0.]
             [0. 0. 0. ... 0. 0. 0.]
             ...
             [0. 0. 0. ... 0. 0. 0.]
             [0. 0. 0. ... 0. 0. 0.]
             [0. 0. 0. ... 0. 0. 0.]], bbox=(minimum=(0, 0), maximum=(99, 99))
            mask=[[0 0 0 ... 0 0 0]
             [0 0 0 ... 0 0 0]
             [0 0 0 ... 0 0 0]
             ...
             [0 0 0 ... 0 0 0]
             [0 0 0 ... 0 0 0]
             [0 0 0 ... 0 0 0]], bbox=(minimum=(0, 0), maximum=(99, 99)), maskPlaneDict={'BAD': 0, 'CR': 3, 'DETECTED': 5, 'DETECTED_NEGATIVE': 6, 'EDGE': 4, 'INTRP': 2, 'NO_DATA': 8, 'SAT': 1, 'SUSPECT': 7}
            variance=[[0. 0. 0. ... 0. 0. 0.]
             [0. 0. 0. ... 0. 0. 0.]
             [0. 0. 0. ... 0. 0. 0.]
             ...
             [0. 0. 0. ... 0. 0. 0.]
             [0. 0. 0. ... 0. 0. 0.]
             [0. 0. 0. ... 0. 0. 0.]], bbox=(minimum=(0, 0), maximum=(99, 99))
            

            Pair coded with gkovacs and krzys.

            Parejkoj John Parejko added a comment - tjenness , price : Since you both expressed opinions about this, I figured you'd want to review it. It delegates the array collapsing to numpy, and includes the bbox. I think that means it also tells you the xy0 (as the minimum of the bbox): we weren't able to come up with a test case that behaved otherwise, but maybe you know of one? Here's what an example str(maskedImage) looks like. >>> print(lsst.afw.image.MaskedImage(100,100)) image=[[0. 0. 0. ... 0. 0. 0.] [0. 0. 0. ... 0. 0. 0.] [0. 0. 0. ... 0. 0. 0.] ... [0. 0. 0. ... 0. 0. 0.] [0. 0. 0. ... 0. 0. 0.] [0. 0. 0. ... 0. 0. 0.]], bbox=(minimum=(0, 0), maximum=(99, 99)) mask=[[0 0 0 ... 0 0 0] [0 0 0 ... 0 0 0] [0 0 0 ... 0 0 0] ... [0 0 0 ... 0 0 0] [0 0 0 ... 0 0 0] [0 0 0 ... 0 0 0]], bbox=(minimum=(0, 0), maximum=(99, 99)), maskPlaneDict={'BAD': 0, 'CR': 3, 'DETECTED': 5, 'DETECTED_NEGATIVE': 6, 'EDGE': 4, 'INTRP': 2, 'NO_DATA': 8, 'SAT': 1, 'SUSPECT': 7} variance=[[0. 0. 0. ... 0. 0. 0.] [0. 0. 0. ... 0. 0. 0.] [0. 0. 0. ... 0. 0. 0.] ... [0. 0. 0. ... 0. 0. 0.] [0. 0. 0. ... 0. 0. 0.] [0. 0. 0. ... 0. 0. 0.]], bbox=(minimum=(0, 0), maximum=(99, 99)) Pair coded with gkovacs and krzys .
            Parejkoj John Parejko added a comment - Jenkins run: https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/28903/pipeline
            price Paul Price added a comment -

            Looking only at the example, right now:

            • Can you get rid of the duplicated bbox entries (probably requires special-casing MaskedImage).
            • I think the class name should be in there somewhere, (MaskedImage(...))?
            • It would be good to include information about the pixel type, either by making this explicit (e.g., dtype=float32) or through the class name (e.g., MaskedImageF).
            • Should have commas between the image, mask and variance elements.
            • Does __repr__ work the same way?
            price Paul Price added a comment - Looking only at the example, right now: Can you get rid of the duplicated bbox entries (probably requires special-casing MaskedImage ). I think the class name should be in there somewhere, ( MaskedImage(...) )? It would be good to include information about the pixel type, either by making this explicit (e.g., dtype=float32 ) or through the class name (e.g., MaskedImageF ). Should have commas between the image , mask and variance elements. Does __repr__ work the same way?
            tjenness Tim Jenness added a comment -

            Usually repr should show the class but str tends not to. Not that we've been overly consistent with this.

            tjenness Tim Jenness added a comment - Usually repr should show the class but str tends not to. Not that we've been overly consistent with this.
            tjenness Tim Jenness added a comment -

            Pulling out of review since both reviewers have commented.

            tjenness Tim Jenness added a comment - Pulling out of review since both reviewers have commented.
            Parejkoj John Parejko added a comment - - edited

              Can you get rid of the duplicated bbox entries (probably requires special-casing MaskedImage).

            Our thought with this one was that we didn't see explicit checks that the three components of a MaskedImage had to have the same bbox. But I just tested it, and it looks like you can't make a MaskedImage with different bboxes with the (Image, Mask, Variance) constructor. So, I'll special case and get rid of the extra bbox printing. Unfortunate, since it would be nice to keep them in sync for future changes.

            • I think the class name should be in there somewhere, (MaskedImage(...))?
            • It would be good to include information about the pixel type, either by making this explicit (e.g., dtype=float32) or through the class name (e.g., MaskedImageF).
            • Does _repr_ work the same way?

            As tjenness says, that's what repr is for, and what it does, in this case (prepends the namespace and class name). For comparison, look at repr vs. str for a numpy array.

            Should have commas between the image, mask and variance elements.

            Done.

            Parejkoj John Parejko added a comment - - edited   Can you get rid of the duplicated bbox entries (probably requires special-casing MaskedImage ). Our thought with this one was that we didn't see explicit checks that the three components of a MaskedImage had to have the same bbox. But I just tested it, and it looks like you can't make a MaskedImage with different bboxes with the (Image, Mask, Variance) constructor. So, I'll special case and get rid of the extra bbox printing. Unfortunate, since it would be nice to keep them in sync for future changes. I think the class name should be in there somewhere, ( MaskedImage(...) )? It would be good to include information about the pixel type, either by making this explicit (e.g., dtype=float32 ) or through the class name (e.g., MaskedImageF ). Does _ repr _ work the same way? As tjenness says, that's what repr is for, and what it does, in this case (prepends the namespace and class name). For comparison, look at repr vs. str for a numpy array. Should have commas between the image , mask and variance elements. Done.
            Parejkoj John Parejko added a comment -

            Thanks for the review. Merged and done.

            Parejkoj John Parejko added a comment - Thanks for the review. Merged and done.

            People

              Parejkoj John Parejko
              Parejkoj John Parejko
              Paul Price, Tim Jenness
              Jim Bosch, John Parejko, John Swinbank, Paul Price, Russell Owen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.