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-2146 - select correct asset from stac Item #14

Merged
merged 11 commits into from
May 30, 2024

Conversation

flamingbear
Copy link
Member

@flamingbear flamingbear commented May 28, 2024

Description

Adds a function to BrowseImageGeneratorAdapter to select correct asset for generating browse images when multiple files are present in the granule.

Though a combination of updating the UMM-G record with a new subtype of "BROWSE IMAGE SOURCE" and HARMONY-1751. Harmony can now present assets that include a role of 'visual'.

"browse": {
      "href": "https://archive.swot.podaac.earthdata.nasa.gov/podaac-swot-ops-cumulus-protected/SWOT_L2_HR_LakeSP_1.0/SWOT_L2_HR_LakeSP_Unassigned_407_008_SI_20230121T143155_20230121T143229_PIA0_01.zip",
      "title": "SWOT_L2_HR_LakeSP_Unassigned_407_008_SI_20230121T143155_20230121T143229_PIA0_01.zip",
      "description": "Download SWOT_L2_HR_LakeSP_Unassigned_407_008_SI_20230121T143155_20230121T143229_PIA0_01.zip",
      "roles": [
        "visual"
      ]
    }

HyBIG has been modified to select an assets with role 'visual' before falling back to a role 'data' as before.

Jira Issue ID

DAS-2146

Local Test Steps

Pull this branch, build and run the tests.

❯ ./bin/build-image && ./bin/build-test && ./bin/run-test

If those pass, you can deploy to your Harmony-In-A-Box and run the regression tests against localhost.

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.
  • [n/a] Documentation updated (if needed).

flamingbear and others added 6 commits May 20, 2024 13:38
Verified with tests and regression tests.
This probably works, but it is a first try.
Tests each case for finding correct asset in Item.
Also updates changelog with info on how to keep it up to date and adds links to
version diffs.
@flamingbear flamingbear marked this pull request as ready for review May 28, 2024 22:40
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.

The changes look good. I only had super minor nits on the code and tests.

The other quick question: is this change meant to trigger an associated release? If so, it doesn't look like changes to docker/service_version.txt and CHANGELOG.md got pushed up to remote.

harmony_browse_image_generator/adapter.py Outdated Show resolved Hide resolved
tests/unit/test_adapter.py Outdated Show resolved Hide resolved
@flamingbear
Copy link
Member Author

The other quick question: is this change meant to trigger an associated release? If so, it doesn't look like changes to docker/service_version.txt and CHANGELOG.md got pushed up to remote.

Screenshot 2024-05-29 at 4 39 58 PM

When I added the link to the version diffs in github, I neglected to correct
the script that pulls out the changelog information.
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.

I built the image and ran the tests successfully, then in turn locally executed the HyBIG_Regression.ipynb notebook, which was also successful.

The code looked pretty straightforward to me. I just noted a couple typos.

harmony_browse_image_generator/adapter.py Outdated Show resolved Hide resolved
tests/unit/test_adapter.py Outdated Show resolved Hide resolved
@flamingbear flamingbear merged commit bad8970 into main May 30, 2024
4 checks passed
@flamingbear flamingbear deleted the mhs/DAS-2146/read-browse-image-source branch May 30, 2024 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants