-
Notifications
You must be signed in to change notification settings - Fork 5
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
Conversation
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.
Updates pip_test_requirements.txt Switches the coverage call.
There was a problem hiding this 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.
Also adds back __init__.py to get tests running locally. (outside of docker and workflows)
@owenlittlejohns I think I addressed all of your concerns or responded to your comments. This is ready for re-review I think. |
There was a problem hiding this 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)
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. |
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 |
D'oh. If in doubt, I should try reading things. Sorry about that. Retrying now. |
There was a problem hiding this 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!
Description
create_browse
from the libraryis 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.
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.
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:
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.
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.
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 workflowsVerify 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
CHANGELOG.md
updated to include high level summary of PR changes.docker/service_version.txt
updated if publishing a release.