Skip to content

Commit

Permalink
Change national risk location link
Browse files Browse the repository at this point in the history
  • Loading branch information
b-j-mills committed Feb 7, 2024
1 parent d5df2d8 commit f4d09b8
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 6 deletions.
6 changes: 6 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.6.1]

### Changed

- National risk location link

## [0.6.0]

### Added
Expand Down
28 changes: 23 additions & 5 deletions src/hapi_schema/db_national_risk.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
)
from sqlalchemy.orm import Mapped, mapped_column, relationship

from hapi_schema.db_admin1 import DBAdmin1
from hapi_schema.db_admin2 import DBAdmin2
from hapi_schema.db_dataset import DBDataset
from hapi_schema.db_location import DBLocation
from hapi_schema.db_resource import DBResource
Expand All @@ -27,8 +29,8 @@ class DBNationalRisk(Base):
ForeignKey("resource.id", onupdate="CASCADE", ondelete="CASCADE"),
nullable=False,
)
location_ref: Mapped[int] = mapped_column(
ForeignKey("location.id", onupdate="CASCADE"), nullable=False
admin2_ref: Mapped[int] = mapped_column(
ForeignKey("admin2.id", onupdate="CASCADE"), nullable=False
)
risk_class: Mapped[int] = mapped_column(Integer, nullable=False)
global_rank: Mapped[int] = mapped_column(Integer, nullable=False)
Expand All @@ -51,7 +53,7 @@ class DBNationalRisk(Base):
source_data: Mapped[str] = mapped_column(Text, nullable=True)

resource = relationship("DBResource")
location = relationship("DBLocation")
admin2 = relationship("DBAdmin2")


view_params_national_risk = ViewParams(
Expand All @@ -69,11 +71,27 @@ class DBNationalRisk(Base):
DBResource.update_date.label("resource_update_date"),
DBLocation.code.label("location_code"),
DBLocation.name.label("location_name"),
DBAdmin1.code.label("admin1_code"),
DBAdmin1.name.label("admin1_name"),
DBAdmin1.is_unspecified.label("admin1_is_unspecified"),
DBAdmin2.code.label("admin2_code"),
DBAdmin2.name.label("admin2_name"),
DBAdmin2.is_unspecified.label("admin2_is_unspecified"),
).select_from(
# Join risk to loc
# Join risk to admin 2 to admin 1 to loc
DBNationalRisk.__table__.join(
DBAdmin2.__table__,
DBNationalRisk.admin2_ref == DBAdmin2.id,
isouter=True,
)
.join(
DBAdmin1.__table__,
DBAdmin2.admin1_ref == DBAdmin1.id,
isouter=True,
)
.join(
DBLocation.__table__,
DBNationalRisk.location_ref == DBLocation.id,
DBAdmin1.location_ref == DBLocation.id,
isouter=True,
)
# Join risk to resource to dataset
Expand Down
2 changes: 1 addition & 1 deletion tests/sample_data/data_national_risk.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
dict(
id=1,
resource_ref=1,
location_ref=1,
admin2_ref=1,
risk_class=5,
global_rank=4,
overall_risk=8.1,
Expand Down
2 changes: 2 additions & 0 deletions tests/test_national_risk_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ def test_national_risk_view(run_view_test):
== "c3f001fa-b45b-464c-9460-1ca79fd39b40",
view_national_risk.c.resource_hdx_id
== "90deb235-1bf5-4bae-b231-3393222c2d01",
view_national_risk.c.admin2_code == "FOO-XXX-XXX",
view_national_risk.c.admin1_code == "FOO-XXX",
view_national_risk.c.location_code == "FOO",
),
)

3 comments on commit f4d09b8

@davidmegginson
Copy link
Member

Choose a reason for hiding this comment

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

@b-j-mills -- the original intention was to use location_ref rather than admin2_ref, since this is national-level data. We should discuss (if it's easier to keep it consistent, we can change the spec).

@b-j-mills
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@davidmegginson I originally wrote it to use location_ref, but to be consistent with the population data I switched to admin2_ref. The population data exists at admin0, admin1, and admin2 and uses admin2_ref for all three admin levels. I can change it if needed! I thought it would be easier from the query side as well if they all referred to admin codes in the same way.

@davidmegginson
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the reply, @b-j-mills — that makes sense.

The question is whether we'll ever use this table for subnational risk, or we'll create a new table for that. If this table will always be for national risk, then I suggest going back to location_ref so that anyone looking at it in the future doesn't get confused; there won't be any extra query complexity, because the API uses the view regardless. But if we want to leave the door open for using the same table for subnational risk at some point in the future, then admin2_ref is also fine.

Please sign in to comment.