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-2161: Updates libs with no user visible changes. #16

Merged
merged 12 commits into from
Jun 10, 2024
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ HyBIG follows semantic versioning. All notable changes to this project will be
documented in this file. The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).

## [unreleased] - 2024-06-06

### Changed
Updated internal library dependencies.

## [v1.2.0] - 2024-05-28

Expand Down
3 changes: 2 additions & 1 deletion harmony_browse_image_generator/browse.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
TRANSPARENT,
TRANSPARENT_IDX,
TRANSPARENT_RGBA,
all_black_color_map,
get_color_palette,
remove_alpha,
)
Expand Down Expand Up @@ -286,7 +287,7 @@ def get_color_map_from_image(image: Image) -> dict:

"""
color_tuples = np.array(image.getpalette(rawmode='RGBA')).reshape(-1, 4)
color_map = {}
color_map = all_black_color_map()
Copy link
Member

Choose a reason for hiding this comment

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

Okay then. This is me gathering my thoughts (please tell me if anything below is wonky):

  • all_black_color_map returns a dictionary of 256 key/value pairs. All have an RGBA tuple that is black.
  • Below this line you then iterate through all the values in color_tuples and assign those values to their appropriate index in the dictionary (but it essentially starts at 0 and fills the first N indices).

So, the questions I have:

  • Will you ever get a time when color_tuples is significantly less than 256, and you have multiple black values at the end of the colour map? Does that matter?
  • Will you ever get a time when colour_tuples is more than 256? Would that be a problem? (It seems like the code will all still work, but maybe nodata would not be set as expected?)

Possibly nitpicky thought (feel free to ignore), you could use enumerate below maybe:

for idx, color_tuple in enumerate(color_tuples):
    color_map[idx] = tuple(color_tuple)

(And yes, I know I used "color", but only to be consistent 😉)

Copy link
Member Author

@flamingbear flamingbear Jun 6, 2024

Choose a reason for hiding this comment

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

Will you ever get a time when color_tuples is significantly less than 256, and you have multiple black values at the end of the colour map? Does that matter?

Yes, we have gotten that before and will still see that. It does not matter, because there are no values in the data that will have those indexes. For example run gdalinfo on the ASTGTMV003_N00E022_dem.png from the regression tests.

Will you ever get a time when colour_tuples is more than 256? Would that be a problem? (It seems like the code will all still work, but maybe nodata would not be set as expected?)

No, because we only ask for 254 values when quantizing .

I just tried with 300 for giggles and quantize won't go above 256: ValueError: bad number of colors

Also No Data is set right before reprojection, so it would end up overwriting the last color in the table. Which is also why I only ask for 254 values, so that we have a transparent and no data index available without ruining any of the quantized values.

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly nitpicky thought (feel free to ignore), you could use enumerate below maybe:

for idx, color_tuple in enumerate(color_tuples):
    color_map[idx] = tuple(color_tuple)

Sure that's cleaner.

for idx in range(0, color_tuples.shape[0]):
color_map[idx] = tuple(color_tuples[idx])
return color_map
Expand Down
5 changes: 5 additions & 0 deletions harmony_browse_image_generator/color_utility.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ def get_remote_palette_from_source(source: HarmonySource) -> dict:
raise HyBIGNoColorInformation('No color in source') from exc


def all_black_color_map():
"""Return a full length rgba color map with all black values."""
return {idx: (0, 0, 0, 255) for idx in range(256)}


def convert_colormap_to_palette(colormap: dict) -> ColorPalette:
"""Convert a GeoTIFF palette to GDAL ColorPalette.
Expand Down
14 changes: 7 additions & 7 deletions pip_requirements.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
harmony-service-lib~=1.0.26
harmony-service-lib~=1.0.27
pystac~=0.5.6
matplotlib==3.7.1
rasterio==1.3.6
rioxarray==0.15.0
numpy==1.24.2
pillow==10.0.0
pyproj==3.6.0
matplotlib==3.9.0
rasterio==1.3.10
rioxarray==0.15.5
numpy==1.26.4
pillow==10.3.0
pyproj==3.6.1
13 changes: 8 additions & 5 deletions tests/unit/test_browse.py
Original file line number Diff line number Diff line change
Expand Up @@ -631,11 +631,14 @@ def test_get_color_map_from_image(self):
test_image.putpalette(palette_sequence, rawmode='RGBA')

expected_color_map = {
0: (255, 0, 0, 255),
1: (0, 255, 0, 255),
2: (0, 0, 255, 255),
3: (225, 100, 25, 25),
4: (0, 0, 0, 0),
**{
0: (255, 0, 0, 255),
1: (0, 255, 0, 255),
2: (0, 0, 255, 255),
3: (225, 100, 25, 25),
4: (0, 0, 0, 0),
},
**{idx: (0, 0, 0, 255) for idx in range(5, 256)},
}

actual_color_map = get_color_map_from_image(test_image)
Expand Down
Loading