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

Add 5 new feature road classes to centreline #1157

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add 5 new feature road classes to centreline #1157

wants to merge 6 commits into from

Conversation

chmnata
Copy link
Collaborator

@chmnata chmnata commented Feb 25, 2025

What this pull request accomplishes:

  • Add 5 new feature road classes for bigdata ONLY
  • new gis_core.centreline_latest_all_feature mat view
  • modified gis_core.centreline_latest mat view to include 3 more feature

Issue(s) this solves:

What, in particular, needs to reviewed:

  • updated gcc puller function, dag, dag variable, sql for mat view, readme

What needs to be done by a sysadmin after this PR is merged

  • create the new centreline latest all feature mat view
  • update trigger
  • drop all dependency on centreline_latet and recreate......
  • git pull data script and update symlink

@chmnata chmnata requested a review from gabrielwol February 25, 2025 16:41
@chmnata chmnata linked an issue Feb 25, 2025 that may be closed by this pull request
3 tasks
Copy link
Collaborator

@gabrielwol gabrielwol left a comment

Choose a reason for hiding this comment

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

Looks good, just some minor changes.

@@ -93,17 +101,17 @@ def pull_layer(layer, conn_id):
context["table_name"] = layer[0]
#get db connection
conn = PostgresHook(conn_id).get_conn()

print(layer[1].get("include_additional_feature"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove

Comment on lines +655 to +656
LOGGER.info("include_additional_feature flag: %s", include_additional_feature)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove

Comment on lines +319 to +321
where = "\"FEATURE_CODE_DESC\" IN ('Collector','Collector Ramp','Expressway','Expressway Ramp','Local','Major Arterial','Major Arterial Ramp','Minor Arterial','Minor Arterial Ramp','Pending', 'Other', 'Trail', 'Busway', 'Laneway', 'Other Ramp', 'Access Road')"
else:
where = "\"FEATURE_CODE_DESC\" IN ('Collector','Collector Ramp','Expressway','Expressway Ramp','Local','Major Arterial','Major Arterial Ramp','Minor Arterial','Minor Arterial Ramp','Pending', 'Other')"
Copy link
Collaborator

Choose a reason for hiding this comment

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

To make this section a bit more clear about what's being added, could you just have the if include_additional_feature: clause append OR feature_code_desc IN (...) to the default?

| Local | ✅ | ✅ |
| Pending | ✅ | ✅ |
| Other (added `2024-02-19`) | ✅ | ✅ |
| Buslane (added `2025-02-24`) | ❌ | ✅ |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Busway

'Other',
'Other Ramp',
'Laneway',
'Pending');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a mat view comment specifying which layers are left out?

@@ -14,7 +14,8 @@ The centreline data are used by many other groups in the City and it's often imp

## How It's Structured

* The `gis_core.centreline_latest` Materialized View contains the latest set of lines with unique id `centreline_id`. These lines are undirected. All edges have _from_ and _to_ nodes, though this should not be taken to indicate that edges are directed. For a directed centreline layer, check out `gis_core.routing_centreline_directional` ([see more](#centreline-segments-edges)) which has the necessary schema to be used in pg_routing.
* The `gis_core.centreline_latest` Materialized View contains the latest set of lines for road classes that are relevant to transportation. It includes all road classification but **excludes** `Trail` and `Buslane`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Busway

* 'Pending'
* 'Other' (version >= `2024-02-19`)
> [!IMPORTANT]
> **2025-02-24**: Added `Buslane`, `Trail`, `Access Road`, `Other Ramp`, and `Laneway`, in order to ensure consistency with MOVE.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Busway

@gabrielwol
Copy link
Collaborator

Also we should remember to keep Bancroft gcc_puller up to date, although these changes won't affect it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GCC puller: update centreline filter
2 participants