Mhs/das 2108/handle no data #6
Conversation
test_convert_singleband_to_raster_without_colortable is updated to use a data array directly as well as include a missing data check. Missing data coded as np.nan is replaced by the TRANSPARENT_RGBA value (0,0,0,0)
Test Cleanup, better naming.
There was a problem hiding this comment.
Nice work! I ran the example from the description and instead of getting entirely black tiles, I got mostly transparent tiles with trajectories/swaths of populated data on them.
I had one question (that probably won't lead to any changes) about the range, but that's mainly for my own understanding.
| @@ -7,6 +7,8 @@ | |||
|
|
|||
| import requests | |||
| from harmony.message import Source as HarmonySource | |||
| import numpy as np | |||
| from numpy import ndarray | |||
There was a problem hiding this comment.
Nit: It feels a bit weird importing all of numpy and then also separately np.ndarray. Not a big deal, but just feels a bit odd.
There was a problem hiding this comment.
Yup. I'm surprised I can't have ndarray in a TYPE_CHECKING guard, because that's all it's used for. I can change them all to np.ndarray, but I remember the format got black wonky.
Ok, I tried again. I can update it by putting my type in single quotes. Is that better?
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from numpy import ndarrayAnd then
def remove_alpha(raster: 'ndarray') -> tuple['ndarray', 'ndarray', None]:
"""remove alpha layer when it exists."""
if raster.shape[0] == 4:
return raster[0:3, :, :], raster[3, :, :]
return raster, NoneThere was a problem hiding this comment.
So, I'm worried this next comment is me missing something, but couldn't you just stick with the single import (import numpy as np) and then:
def remove_alpha(raster: np.ndarray) -> tuple[np.ndarray, np.ndarray, None]:
I tried a toy model locally, and that seemed okay.
There was a problem hiding this comment.
I could. This was why I hadn't elsewhere: "but I remember the format got black wonky"
There was a problem hiding this comment.
but I don't have strong opinions I suppose 9a7902f
| from osgeo_utils.auxiliary.color_palette import ColorPalette | ||
| from PIL import Image | ||
| from rasterio.io import DatasetReader | ||
| from rasterio.plot import reshape_as_image, reshape_as_raster | ||
| from rasterio.warp import Resampling, reproject | ||
| from rioxarray import open_rasterio | ||
| from xarray import DataArray | ||
| from numpy import ndarray |
There was a problem hiding this comment.
Huh - weird that this ordering happened. Feels a bit off, but I guess I trust it.
There was a problem hiding this comment.
I noticed this too*, I think we don't have that set in the pre-commit.
- and then I forgot to check it.
There was a problem hiding this comment.
I had a bad pyproject.toml, not sure why our precommit didn't catch it though. I will look
There was a problem hiding this comment.
I think we should propagate this to the other repos as we touch them.
da4a015
There was a problem hiding this comment.
Good catch - I might just put up a couple of PRs now. (Some of those repos don't get much love and we might forget)
|
@owenlittlejohns You want to re-review the latest changes? |
|
|
||
| expected_raster = np.array( | ||
| [ | ||
| [ | ||
| [0, 104, 198, 255], | ||
| [0, 0, 198, 255], |
There was a problem hiding this comment.
Did you change this from 104 to 0 to demonstrate transparent pixel behavior?
There was a problem hiding this comment.
Well, yes, this is the expected array, I added a missing value at the [0,1] location, and these changes are the ones reflected in the output from the missing data.
There was a problem hiding this comment.
Oh right. You changed the code so the expected array changed.
|
I ran the test using the test-prefire.py script, and it looks like I got the expected output. The left image shows the original problem, and the right image is the corrected output using this branch. 
I added a few questions, but they're mainly just for my own understanding. Cool stuff! |
Description
This PR updates HyBIG to appropriately handle masked data. Rioxarray's mask_and_scale masks bad data some details here
This PR ensures that data that comes in as "bad" according to rioxarray is transparent, i.e. not shown, in the final output image.
This corrects a bug where data with nans was normalized incorrectly leading to all black output images.
Code and tests were updated to ensure any image, 1 band raster without associated colortable, 1band with colortable, a 3band raster and 4band raster are all tested with missing/bad data.
Jira Issue ID
DAS-2108
Local Test Steps
build and run the image and tests.
Deploy the new image to your local Harmony-In-A-Box.
This has been associated with HGA again and can't be tested in harmony at the moment.
Ensure you're on the VPN, associate HyBIG with C1260737748-ASDC_DEV2 and Test PREFIRE data with this command:http://localhost:3000/C1260737748-ASDC_DEV2/ogc-api-coverages/1.0.0/collections/all/coverage/rangeset?forceAsync=true&granuleId=G1262401617-ASDC_DEV2&format=image%2Fpng&outputCrs=EPSG%3A4326&skipPreview=trueAlternatively download a test file here:
https://data.asdc.sit.earthdata.nasa.gov/asdc2-sit-protected/browse/PREFIRE/PREFIRE_SAT2_2B-FLX_R00/2021.07.21/PREFIRE_SAT2_2B-FLX_S07_R00_20210721044449_03042.nc.G00.tif
And run it through this gist.
https://gist.github.com/flamingbear/ce1a41962a65766c5230bab736cc1002
visualize the output in qgis.
PR Acceptance Checklist
CHANGELOG.mdupdated to include high level summary of PR changes.docker/service_version.txtupdated if publishing a release.