Merge pull request #332 from nsidc/render-null-performance-criticality

Render null performance criticality

CHANGELOG.md

@@ -1,3 +1,7 @@
+## v2.3.0 (2024-10-02)
+
+* Allow null link raings (db migration ea1181510e10)
+
 ## v2.2.0 (2024-10-01)
 
 * Allow nodes of same type to link.

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:

VERSION.env

@@ -1 +1 @@
-export USAON_BENEFIT_TOOL_VERSION="v2.2.0"
+export USAON_BENEFIT_TOOL_VERSION="v2.3.0"

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

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 ###

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 = "[email protected]:nsidc/usaon-benefit-tool.git"
 authors = [
   {name = "National Snow and Ice Data Center", email = "[email protected]"},
@@ -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,17 +96,17 @@ 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]
-current_version = "2.2.0"
+current_version = "2.3.0"
 commit = false
 tag = false
 

usaon_benefit_tool/constants/colormap.py

@@ -16,3 +16,4 @@
     cmap.colors,  # type: ignore [attr-defined]
     N=COLORMAP_CLASSES,
 )
+COLOR_NO_PERFORMANCE_RATING = "#666666"

usaon_benefit_tool/constants/version.py

@@ -1 +1 @@
-VERSION = "2.2.0"
+VERSION = "2.3.0"

usaon_benefit_tool/models/tables.py

@@ -402,15 +402,15 @@ class Link(BaseModel):
             'performance_rating>0 and performance_rating<101',
             name='p1-100',
         ),
-        nullable=False,
+        nullable=True,
     )
     criticality_rating = Column(
         Integer,
         CheckConstraint(
             '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)

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.

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)

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,
@@ -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.
 
@@ -132,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(
@@ -211,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) -> int | float:
+    """If criticality rating is not set, return a very thin line."""
+    if criticality_rating is None:
+        return 0.1
+
+    return criticality_rating