From 4dcc8f3759cdc9b2d13b9d1bf1220e973b9f4a62 Mon Sep 17 00:00:00 2001 From: Robyn Marowitz Date: Wed, 2 Oct 2024 15:23:50 -0600 Subject: [PATCH 01/11] update table to allow nulls for link ratings --- usaon_benefit_tool/models/tables.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/usaon_benefit_tool/models/tables.py b/usaon_benefit_tool/models/tables.py index 5709780b..cbb2f748 100644 --- a/usaon_benefit_tool/models/tables.py +++ b/usaon_benefit_tool/models/tables.py @@ -402,7 +402,7 @@ class Link(BaseModel): 'performance_rating>0 and performance_rating<101', name='p1-100', ), - nullable=False, + nullable=True, ) criticality_rating = Column( Integer, @@ -410,7 +410,7 @@ class Link(BaseModel): 'criticality_rating>0 and criticality_rating<11', name='c1-10', ), - nullable=False, + nullable=True, ) performance_rating_rationale = Column(String(8192), nullable=True) critically_rating_rationale = Column(String(8192), nullable=True) From 2ad292f70b89590eb7c4e41578ac8aa8b4289540 Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Wed, 2 Oct 2024 15:26:13 -0600 Subject: [PATCH 02/11] Remove unused sankey_subset function --- usaon_benefit_tool/util/sankey.py | 33 ------------------------------- 1 file changed, 33 deletions(-) diff --git a/usaon_benefit_tool/util/sankey.py b/usaon_benefit_tool/util/sankey.py index b1fc4322..c8f59e5e 100644 --- a/usaon_benefit_tool/util/sankey.py +++ b/usaon_benefit_tool/util/sankey.py @@ -65,39 +65,6 @@ def sankey(assessment: Assessment) -> HighchartsSankeySeries: return series -def sankey_subset( - assessment: Assessment, - include_related_to_type: type[AssessmentNode], -) -> HighchartsSankeySeries: - """Provide subset of Sankey data structure. - - Include only nodes related to `include_related_to_type`. - """ - series = _sankey(assessment) - # FIXME: Using `cls.__name__` could be much better. Replace with an Enum for node - # type. - node_ids_matching_object_type = [ - n["id"] - for n in series["nodes"] - if n["type"] == include_related_to_type.__name__ - ] - - # For now, select only the outputs. That was the previous behavior, but do we like - # it? - filtered_links = [ - link for link in series["data"] if link["from"] in node_ids_matching_object_type - ] - - filtered_node_ids = _node_ids_in_links(filtered_links) - filtered_nodes = [ - node for node in series["nodes"] if node["id"] in filtered_node_ids - ] - return { - "data": filtered_links, - "nodes": filtered_nodes, - } - - def _sankey(assessment: Assessment) -> HighchartsSankeySeries: """Extract Sankey-relevant data from Response and format for Highcharts. From df7dc8205c48a4a19f00517cc9fde8d10e756dbf Mon Sep 17 00:00:00 2001 From: Robyn Marowitz Date: Wed, 2 Oct 2024 15:30:57 -0600 Subject: [PATCH 03/11] DB migration v2.3.0 --- deploy/post/v2.3.0 | 24 +++++++++ ...a1181510e10_allow_null_for_link_ratings.py | 52 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100755 deploy/post/v2.3.0 create mode 100644 migrations/versions/ea1181510e10_allow_null_for_link_ratings.py diff --git a/deploy/post/v2.3.0 b/deploy/post/v2.3.0 new file mode 100755 index 00000000..0158acbc --- /dev/null +++ b/deploy/post/v2.3.0 @@ -0,0 +1,24 @@ +#!/bin/bash + +set -euo pipefail + +# HACK: Garrison doesn't see these variables that were exported in main deploy +# script, which is unintuitive. NOTE that sourcing VERSION.env is not +# compatible with integration, but this script doesn't receive the environment +# as a parameter. Garrison needs some extra thought here. +source /etc/profile.d/envvars.sh +source VERSION.env + +# TODO: figure out better way +echo "Waiting 10 seconds for the new stack to come up..." +sleep 10 + +docker compose run --rm usaon-benefit-tool alembic upgrade head + +# confirm the expected migration was applied +current=$(docker compose run --rm usaon-benefit-tool alembic current 2>/dev/null) +if [ "ea1181510e10 (head)" = "${current}" ]; then + echo "Data migration successful. On expected revision ${current}." +else + echo "Data migration failed. On unexpected revision ${current}." +fi diff --git a/migrations/versions/ea1181510e10_allow_null_for_link_ratings.py b/migrations/versions/ea1181510e10_allow_null_for_link_ratings.py new file mode 100644 index 00000000..1566d9d0 --- /dev/null +++ b/migrations/versions/ea1181510e10_allow_null_for_link_ratings.py @@ -0,0 +1,52 @@ +"""Allow null for link ratings. + +Revision ID: ea1181510e10 +Revises: 925ed377e31b +Create Date: 2024-10-02 21:27:47.043654 + +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = 'ea1181510e10' +down_revision: str | None = '925ed377e31b' +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column( + 'link', + 'performance_rating', + existing_type=sa.INTEGER(), + nullable=True, + ) + op.alter_column( + 'link', + 'criticality_rating', + existing_type=sa.INTEGER(), + nullable=True, + ) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column( + 'link', + 'criticality_rating', + existing_type=sa.INTEGER(), + nullable=False, + ) + op.alter_column( + 'link', + 'performance_rating', + existing_type=sa.INTEGER(), + nullable=False, + ) + # ### end Alembic commands ### From d303a120048b7f7769919e45fd74b0cb81910af7 Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Wed, 2 Oct 2024 15:32:48 -0600 Subject: [PATCH 04/11] Support null (None) values in performance and criticality rating when rendering --- usaon_benefit_tool/constants/colormap.py | 1 + usaon_benefit_tool/util/colormap.py | 11 +++++++++-- usaon_benefit_tool/util/sankey.py | 10 +++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/usaon_benefit_tool/constants/colormap.py b/usaon_benefit_tool/constants/colormap.py index ebcf758f..0887d11b 100644 --- a/usaon_benefit_tool/constants/colormap.py +++ b/usaon_benefit_tool/constants/colormap.py @@ -16,3 +16,4 @@ cmap.colors, # type: ignore [attr-defined] N=COLORMAP_CLASSES, ) +COLOR_NO_PERFORMANCE_RATING = "#666666" diff --git a/usaon_benefit_tool/util/colormap.py b/usaon_benefit_tool/util/colormap.py index 6e041770..638a9018 100644 --- a/usaon_benefit_tool/util/colormap.py +++ b/usaon_benefit_tool/util/colormap.py @@ -6,6 +6,7 @@ from matplotlib.colors import Normalize, rgb2hex from usaon_benefit_tool.constants.colormap import ( + COLOR_NO_PERFORMANCE_RATING, COLORMAP, COLORMAP_DATA_VALUE_RANGE, COLORMAP_PALETTE, @@ -18,8 +19,14 @@ ) -def color_for_performance_rating(performance_rating: int): - """Return the hex color value for the given performance rating.""" +def color_for_performance_rating(performance_rating: int | None): + """Return the hex color value for the given performance rating. + + If not set, return a default non-colormap value. + """ + if performance_rating is None: + return COLOR_NO_PERFORMANCE_RATING + lookup_val = norm(performance_rating) color = COLORMAP(lookup_val) diff --git a/usaon_benefit_tool/util/sankey.py b/usaon_benefit_tool/util/sankey.py index c8f59e5e..61430494 100644 --- a/usaon_benefit_tool/util/sankey.py +++ b/usaon_benefit_tool/util/sankey.py @@ -99,7 +99,7 @@ def _sankey(assessment: Assessment) -> HighchartsSankeySeries: { "from": _node_id(link.source_assessment_node.node), "to": _node_id(link.target_assessment_node.node), - "weight": link.criticality_rating, + "weight": _weight_for_criticality_rating(link.criticality_rating), "color": color_for_performance_rating(link.performance_rating), "id": link.id, "tooltipHTML": render_template( @@ -178,3 +178,11 @@ def _node_ids_in_links(links: list[HighchartsSankeySeriesLink]) -> set[str]: node_ids = set(chain.from_iterable(node_id_tuples)) return node_ids + + +def _weight_for_criticality_rating(criticality_rating: int | None) -> float: + """If criticality rating is not set, return a very thin line.""" + if criticality_rating is None: + return 0.01 + + return criticality_rating From 9e3b5ea2b5122f7677c158ac37c8f314690dd5ed Mon Sep 17 00:00:00 2001 From: Robyn Marowitz Date: Wed, 2 Oct 2024 15:32:53 -0600 Subject: [PATCH 05/11] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7fc9b4f..4eecf4e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## NEXT_VERSION + +* Allow null link raings (db migration ea1181510e10) + ## v2.2.0 (2024-10-01) * Allow nodes of same type to link. From 6f64a0782ec022b31d5836ccd9fa681228c3e14b Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Wed, 2 Oct 2024 15:34:08 -0600 Subject: [PATCH 06/11] Fixup deprecations in ruff config --- pyproject.toml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 0ad78629..6d7347c0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,7 +44,7 @@ module = "usaon_benefit_tool.*" # disallow_incomlete_defs = true -[tool.ruff] +[tool.ruff.lint] target-version = "py311" select = [ "F", # pyflakes @@ -86,7 +86,7 @@ ignore = [ "N806", ] -[tool.ruff.per-file-ignores] +[tool.ruff.lint.per-file-ignores] # E501: Line too long. Long strings, e.g. URLs, are common in config. "usaon_benefit_tool/models/tables.py" = ["A003"] "tasks/format.py" = ["A001"] @@ -96,13 +96,13 @@ ignore = [ "usaon_benefit_tool/__init__.py" = ["E501", "PLR0915"] "migrations/*" = ["INP001"] -[tool.ruff.isort] +[tool.ruff.lint.isort] known-third-party = ["luigi"] -[tool.ruff.mccabe] +[tool.ruff.lint.mccabe] max-complexity = 8 -[tool.ruff.flake8-quotes] +[tool.ruff.lint.flake8-quotes] inline-quotes = "double" [tool.bumpversion] From 366589ae50dae3e46728d8e3c0873a70794d2d5a Mon Sep 17 00:00:00 2001 From: Robyn Marowitz Date: Wed, 2 Oct 2024 15:36:18 -0600 Subject: [PATCH 07/11] Bumpversion -> v2.3.0 --- CHANGELOG.md | 2 +- CITATION.cff | 4 ++-- VERSION.env | 2 +- pyproject.toml | 4 ++-- usaon_benefit_tool/constants/version.py | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eecf4e5..92d8d9e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## NEXT_VERSION +## v2.3.0 (2024-10-02) * Allow null link raings (db migration ea1181510e10) diff --git a/CITATION.cff b/CITATION.cff index 15354f21..c2bdfe5b 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -10,8 +10,8 @@ type: "software" # https://github.com/zenodo/zenodo/issues/2515 license: "MIT" -version: 2.2.0 -date-released: "2024-10-01" +version: 2.3.0 +date-released: "2024-10-02" url: "https://github.com/nsidc/usaon-benefit-tool" authors: diff --git a/VERSION.env b/VERSION.env index 9ec596cc..d73ad286 100644 --- a/VERSION.env +++ b/VERSION.env @@ -1 +1 @@ -export USAON_BENEFIT_TOOL_VERSION="v2.2.0" +export USAON_BENEFIT_TOOL_VERSION="v2.3.0" diff --git a/pyproject.toml b/pyproject.toml index 0ad78629..a964ceb9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [project] name = "usaon_benefit_tool" description = "Gather data for US AON's Value Tree Analysis (Benefit Tool) process" -version = "2.2.0" +version = "2.3.0" url = "git@github.com:nsidc/usaon-benefit-tool.git" authors = [ {name = "National Snow and Ice Data Center", email = "nsidc@nsidc.org"}, @@ -106,7 +106,7 @@ max-complexity = 8 inline-quotes = "double" [tool.bumpversion] -current_version = "2.2.0" +current_version = "2.3.0" commit = false tag = false diff --git a/usaon_benefit_tool/constants/version.py b/usaon_benefit_tool/constants/version.py index 3c00bb49..707faadc 100644 --- a/usaon_benefit_tool/constants/version.py +++ b/usaon_benefit_tool/constants/version.py @@ -1 +1 @@ -VERSION = "2.2.0" +VERSION = "2.3.0" From 2118cd4552c3c17b9be148359c71f8a3d53d787d Mon Sep 17 00:00:00 2001 From: Robyn Marowitz Date: Wed, 2 Oct 2024 15:51:00 -0600 Subject: [PATCH 08/11] Update user guide --- usaon_benefit_tool/templates/user_guide.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/usaon_benefit_tool/templates/user_guide.md b/usaon_benefit_tool/templates/user_guide.md index 1beb2bdf..c60e20a4 100644 --- a/usaon_benefit_tool/templates/user_guide.md +++ b/usaon_benefit_tool/templates/user_guide.md @@ -13,3 +13,5 @@ Something else... ## Step 3 Another thing! + +Note: If a link has criticality rating of `NULL` the corresponding line with be thinner than the lowest possible rating (1) and if there is a performance rating of `NULL` the line will be gray. From 2e311595280c8905497dad09785dafa218421fc1 Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Wed, 2 Oct 2024 15:53:54 -0600 Subject: [PATCH 09/11] Fixup ruff config --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 513dcb4a..5ceec5ea 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,7 +44,7 @@ module = "usaon_benefit_tool.*" # disallow_incomlete_defs = true -[tool.ruff.lint] +[tool.ruff] target-version = "py311" select = [ "F", # pyflakes From a04f64ab6ab410b49155a808fa3c949cf2addf1c Mon Sep 17 00:00:00 2001 From: Matt Fisher Date: Wed, 2 Oct 2024 15:54:06 -0600 Subject: [PATCH 10/11] Fixup type annotation --- usaon_benefit_tool/util/sankey.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/usaon_benefit_tool/util/sankey.py b/usaon_benefit_tool/util/sankey.py index 61430494..27e49137 100644 --- a/usaon_benefit_tool/util/sankey.py +++ b/usaon_benefit_tool/util/sankey.py @@ -31,7 +31,7 @@ def permitted_source_link_types(node_type: NodeType) -> set[NodeType]: { "from": str, "to": str, - "weight": int, + "weight": int | float, "color": str, "id": NotRequired[int], "tooltipHTML": str, @@ -180,7 +180,7 @@ def _node_ids_in_links(links: list[HighchartsSankeySeriesLink]) -> set[str]: return node_ids -def _weight_for_criticality_rating(criticality_rating: int | None) -> float: +def _weight_for_criticality_rating(criticality_rating: int | None) -> int | float: """If criticality rating is not set, return a very thin line.""" if criticality_rating is None: return 0.01 From 43e6a4d8c77638f60fdc56fe7614376f433945c4 Mon Sep 17 00:00:00 2001 From: Robyn Marowitz Date: Wed, 2 Oct 2024 16:02:13 -0600 Subject: [PATCH 11/11] Change line thickness to 0.1 for criticality null --- usaon_benefit_tool/util/sankey.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usaon_benefit_tool/util/sankey.py b/usaon_benefit_tool/util/sankey.py index 27e49137..be1a43c6 100644 --- a/usaon_benefit_tool/util/sankey.py +++ b/usaon_benefit_tool/util/sankey.py @@ -183,6 +183,6 @@ def _node_ids_in_links(links: list[HighchartsSankeySeriesLink]) -> set[str]: def _weight_for_criticality_rating(criticality_rating: int | None) -> int | float: """If criticality rating is not set, return a very thin line.""" if criticality_rating is None: - return 0.01 + return 0.1 return criticality_rating