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-2180 extract lib #28

Merged
merged 40 commits into from
Jul 31, 2024
Merged

DAS-2180 extract lib #28

merged 40 commits into from
Jul 31, 2024

Conversation

flamingbear
Copy link
Member

@flamingbear flamingbear commented Jul 19, 2024

Description

  • Separates out the browse image logic from the harmony service logic.
  • Creates a pip installable library hybig-py on pypi
  • Exports function create_browse from the library
  • Updates README to explain library usage. [and various other updates]
  • The new function, formats input parameters and creates a Harmony Message that
    is passed directly to the function that the service calls.

Jira Issue ID

DAS-2180

Local Test Steps

Build and run the unit tests like normal.

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

Launch Harmony-In-A-Box and run the hybig regression tests pointing them at your local harmony.

Ensure tests still run

Build and test the package.

> pip install build
> python -m build
  • Create a new virtual env that has access to gdal. either conda and install gdal or just ensure you have gdal lib available.

  • Install the library into the env:

❯ pip install dist/hybig_py-2.0.0.tar.gz

Create a test dir:
Download sample file from ticket https://bugs.earthdata.nasa.gov/secure/attachment/102135/PREFIRE_SAT2_2B-FLX_S07_R00_20210721044449_03042.nc.G00.tif
and save this file in it and run.

from hybig import create_browse

create_browse('PREFIRE_SAT2_2B-FLX_S07_R00_20210721044449_03042.nc.G00.tif')

That should generate tiled output by default over the entire earth

Delete that output or move it and run it again, but with some params.

from hybig import create_browse

scale_extent = {"x": {"min": 0, "max": 180}, "y": {"min": 0, "max": 90}}
height = 4000
width = 8000
params = {"scale_extent": scale_extent, "height": height, "width": width}

create_browse('PREFIRE_SAT2_2B-FLX_S07_R00_20210721044449_03042.nc.G00.tif', params)

That should generate a single file in the upper left quadrant.

check CI

For bonus points. you can install https://github.com/nektos/act and test the workflows
Verify that the library tests run for python 3.10 and 3.11.
Nevermind, you can see the tests ran here https://github.com/nasa/harmony-browse-image-generator/actions/runs/10013888890/job/27682447719?pr=28

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).

Removes the /home path from the coverage dir so that you can run the test
script locally outside of docker.
First stab at separating concerns. This now has a
harmony_browse_image_generator as well as a harmony_service_entry.
This may upgrade to node 20
Mostly reformating/capitalization and removing unused imports
I already had done one by hand so also updates version.  We can go back to v2
on PyPI
This makes more sense since entry is tied to Docker.
Also adds first stab at documenation but I will need to re-review.
renames run_tests -> run_service_tests
Adds run_lib_tests and runs the unit tests against a python matrix.
flamingbear and others added 2 commits July 22, 2024 17:41
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.

Looks good. I'm leaving these comments now without approving just while I wrestle with getting my new laptop to have a local Harmony instance for the first time. (I need to install a few things like nvm, etc, and it might take a tiny bit).

This is really great work, and I think I only have nits.

.github/workflows/run_lib_tests.yml Show resolved Hide resolved
.github/workflows/run_service_tests.yml Show resolved Hide resolved
README.md Show resolved Hide resolved
hybig/browse.py Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tests/run_tests.sh Show resolved Hide resolved
tests/test_code_format.py Outdated Show resolved Hide resolved
tests/pip_test_requirements.txt Show resolved Hide resolved
tests/run_tests.sh Show resolved Hide resolved
docker/service.Dockerfile Show resolved Hide resolved
@flamingbear
Copy link
Member Author

@owenlittlejohns I think I addressed all of your concerns or responded to your comments. This is ready for re-review I think.

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.

I ran some of the test instructions from the PR. I got the new HyBIG version to run in Harmony-in-a-Box.

When pip-installing hybig-py I hit the following issue:

      Could not find gdal-config. Make sure you have installed the GDAL native library and development headers.
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.

Should GDAL have been taken care of for me with the packaging? (Possibly user error here)

@flamingbear
Copy link
Member Author

Should GDAL have been taken care of for me with the packaging? (Possibly user error here)

No, I couldn't ever get that to work, but now that you mention it, it's not clear in the docs. I only mention it in the development section of the readme.

@flamingbear
Copy link
Member Author

I guess in the test instructions it says : Create a new virtual env that has access to gdal. either conda and install gdal or just ensure you have gdal lib available.

But it's not explicit in the readme for the lib

@owenlittlejohns
Copy link
Member

Create a new virtual env that has access to gdal

D'oh. If in doubt, I should try reading things. Sorry about that. Retrying now.

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.

Okay - I ran the example pip installation instructions (once I had GDAL locally). It all worked as described. Nice!

@flamingbear flamingbear reopened this Jul 31, 2024
@flamingbear flamingbear merged commit 3d14e70 into main Jul 31, 2024
9 checks passed
@flamingbear flamingbear deleted the mhs/DAS-2180/extract-lib branch July 31, 2024 23:30
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.

2 participants