Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DAS-2259 fix opera rtc browse images. #34

Merged
merged 18 commits into from
Oct 17, 2024
Merged

Conversation

flamingbear
Copy link
Member

@flamingbear flamingbear commented Oct 15, 2024

Description

A well placed, careful, cast to uint8, prevents trying to create PIL images from array with float32 values.

Also updates numpy to v2. because it has the np.min_scalar_type used in debugging.

Jira Issue ID

DAS-2259

Local Test Steps

  • Check out this branch, build and run the tests.
❯ ./bin/build-image && ./bin/build-test && ./bin/run-test
  • Verify all tests pass. Fire up Harmony-In-A-Box and run the regression test notebook against localhost.

  • Examine the notebook explain-opera.pdf attached to the DAS-2259 ticket for explanation of the change.

  • Lastly generate an image for OPERA RTC:

    1. Download the input tif on the DAS-2259 ticket
      OPERA_L2_RTC-S1_T039-082699-IW3_20240909T092236Z_20240909T134139Z_S1A_30_v1.0_rgb.tif

    2. Run the script below (after installing hybig as editable in your environment)

     from hybig import create_browse
     fn = './OPERA_L2_RTC-S1_T039-082699-IW3_20240909T092236Z_20240909T134139Z_S1A_30_v1.0_rgb.tif'
     create_browse(fn, {})
    1. bask in the glow of output PNGs.

PR Acceptance Checklist

  • Jira ticket acceptance criteria met.
  • CHANGELOG.md updated to include high level summary of PR changes.
  • docker/service_version.txt updated if publishing a release.
  • Tests added/updated and passing.
  • Documentation updated (if needed).

I updated all the libs as part of the DAS-2259 ticket, but didn't test it..Oops
This reverts commit 7b9690a.
This prevents np.float32s from breaking PIL.
This is well considered and safe to do based on the notebook attached to the
DAS-2259 ticket.

Update numpy to v2. because it has the np.min_scalar_type used in the notebook.
@flamingbear flamingbear marked this pull request as ready for review October 15, 2024 21:33
@flamingbear flamingbear force-pushed the mhs/DAS-2259/opera-rtc-fixes branch from f79f1e7 to d439fd5 Compare October 15, 2024 21:34
This causes regression errors that we should do separately.
Base automatically changed from mhs/update-libs to main October 16, 2024 17:48
An error occurred while trying to automatically change base from mhs/update-libs to main October 16, 2024 17:48
Copy link
Collaborator

@lyonthefrog lyonthefrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All unit tests and HyBIG_Regression.ipynb tests passedand Ian the little test script and output the expected PNG:
image

It was a simple change, I don't have any more comments or questions, so I'll approve on my end.

Copy link
Member

@owenlittlejohns owenlittlejohns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work - looks good. I also got an image with a transparent region around the data. The unit tests ran and I successfully ran the regression tests against the image I built locally.

It took me a moment to grok the numbers in the expected test output (remembering that the values would be normalised in the band such that they extend from the minimum to the maximum value), but that was just a me thing getting back into the swing of HyBIG.

@flamingbear
Copy link
Member Author

moment to grok the numbers in the expected test output

Yeah, I wasn't sure it was super valuable other than to show the dtypes would work correctly for just that reason. And I just grabbed values from an existing image so they might not be super elucidating.

@flamingbear flamingbear merged commit c0b776d into main Oct 17, 2024
6 checks passed
@flamingbear flamingbear deleted the mhs/DAS-2259/opera-rtc-fixes branch October 17, 2024 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants