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.

Sign up for GitHub

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
Merged

DAS-2161: Updates libs with no user visible changes. #16

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8225754
DAS-2180: Updates libs with no changes.
flamingbear Jun 4, 2024
e23594e
DAS-2180: upgrade pillow with small work around changes
flamingbear Jun 6, 2024
e344889
DAS-2180: Fixes expected color_map test.
flamingbear Jun 6, 2024
834a21a
DAS-2180: update CHANGELOG
flamingbear Jun 6, 2024
63cbf05
DAS-2180: use enumerate to build color_map
flamingbear Jun 6, 2024
25266eb
DAS-2180: Chuck up a .snyk file
flamingbear Jun 7, 2024
e09ebf4
DAS-2180: Upgrade gdal package.
flamingbear Jun 7, 2024
3f6b6fc
DAS-2180: revert gdal version up
flamingbear Jun 7, 2024
53ffaed
DAS-2161: Fix typo on PR
flamingbear Jun 8, 2024
ff2439b
DAS-2180: remove snyk file
flamingbear Jun 8, 2024
749e62c
DAS-2161: conda gdal to 3.9 / snyk to 3.7
flamingbear Jun 10, 2024
b7481c1
DAS-2161: Release these changes
flamingbear Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .snyk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Snyk (https://snyk.io) policy file, patches or ignores known vulnerabilities.
version: v1.25.0
language-settings:
python: "3.7"
Copy link
Member

Choose a reason for hiding this comment

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

Okay - so just vocalising what I think we've got here: the .snyk file is preserving the version of Python that Snyk was previously using prior to the bump to Python 3.9.

This seems like the way to go for now, but it is a bit iffy that we are almost certainly getting different package versions being scanned by Snyk than we are actually installing in the Docker image. (Side note - it's nice that a .snyk file is actually working here, I don't think that was always the case)

4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ environment via conda, and then install the necessary dependencies for the
service within that environment via conda and pip then install the pre-commit hooks.

```
> conda create -name hybig-env python==3.11
> conda create --name hybig-env python==3.11
> conda install --file conda_requirements.txt
> pip install -r pip_requirements.txt
> pip install -r dev-requirements.txt
Expand Down
2 changes: 1 addition & 1 deletion conda_requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
gdal==3.6.3
gdal==3.9
7 changes: 4 additions & 3 deletions harmony_browse_image_generator/browse.py
View file
Open in desktop
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,9 +287,9 @@ def get_color_map_from_image(image: Image) -> dict:

"""
color_tuples = np.array(image.getpalette(rawmode='RGBA')).reshape(-1, 4)
color_map = {}
for idx in range(0, color_tuples.shape[0]):
color_map[idx] = tuple(color_tuples[idx])
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 •
edited
Loading

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, color_tuple in enumerate(color_tuples):
color_map[idx] = tuple(color_tuple)
return color_map


Expand Down
5 changes: 5 additions & 0 deletions harmony_browse_image_generator/color_utility.py
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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