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

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

    XMLWordPrintable

    Details

    • Story Points:
      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

          relates to

          Bug - A problem which impairs or prevents the functions of the product. DM-16291 str(Image) tests too strict about formatting

          • Done

            Activity

            Ascending order - Click to sort in descending order
            5 older comments
            Hide
            Permalink
            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?
            Show
            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?
            Hide
            Permalink
            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.

            Show
            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.
            Hide
            Permalink
            tjenness Tim Jenness added a comment -

            Pulling out of review since both reviewers have commented.

            Show
            tjenness Tim Jenness added a comment - Pulling out of review since both reviewers have commented.
            Hide
            Permalink
            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 Tim Jenness 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.

            Show
            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 Tim Jenness 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.
            Hide
            Permalink
            Parejkoj John Parejko added a comment -

            Thanks for the review. Merged and done.

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

              People

              Assignee:
              Parejkoj John Parejko
              Reporter:
              Parejkoj John Parejko
              Reviewers:
              Paul Price, Tim Jenness
              Watchers:
              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.