Refactor Scenarios to improve legibility and workflow management. #315

e-lo · 2023-03-07T23:55:50Z

Resolves issues with Scenario class as discussed in #277 including:

removes state variables
improves legibility for what is happening

Updated Definitions

Scenario.project_cards: Mapping[ProjectCard.name,ProjectCard] Storage of all project cards by name; helps prevent naming conflicts.
Scenario._planned_projects: Collection[ProjectCard.name] Initial ordering of project cards which are awaiting to be queued for adding. Similar to a git working directory, project cards aren't recognized in this collection once they are moved to staging
Scenario._queued_projects: Collection[ProjectCard.name] Projects which are "shovel ready": have had pre-requistes checked and done any required re-ordering. Similar to a git staging, project cards aren't recognized in this collection once they are moved to applied
Scenario.queued_projects: @Property wrapper on _queued_projects which lazily queues (checks requirements/conflicts and orders) anything already in Scenario._queued_projects and anything in Scenario.planned_projects (and removes queued projects from planned_projects)
Scenario.applied_projects: Collection[ProjectCard.name] Projects which have been applied to the network.
No more "state" variables: requisites_checked and conflicts_checked: as this is the definitional difference between planned and queued
Scenario.projects: Sum of applied, queued.

To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1204284429821333

Changes how scenarios works per description in #277 and updates relevant documentation about API changes. Also: - various efficiencies and consistency fixes

So you aren't iterating over a changing list

e-lo · 2023-03-10T23:47:27Z

@i-am-sijia or @lmz would love your attention on this next as it will enables some good stuff going on in network-wrangler/projectcard

…into change_project_apply_handling

…ly_handling

e-lo · 2023-03-16T16:53:10Z

@lmz see updated documentation to review!

i-am-sijia

Ran the scenario creation notebook and tests\test_scenario.py, both are currently failing due to bugs in scenario.py
Link and Node foreign keys has upper case variables, but the project card reading step is converting everything in project cards into lower case. This is causing most of the tests to fail.

network_wrangler/projectcard.py

network_wrangler/scenario.py

notebook/Scenario Building Example.ipynb

network_wrangler/scenario.py

Co-authored-by: Sijia Wang <[email protected]>

Also: - Better document Scenario.__init__ arguments - Require minimum variables in base_scenario: applied_projects and conflicts

…p-sag/network_wrangler into change_project_apply_handling

- is_disjoint is actually isdisjoint (doh!) and performed on a set object - mapping objects converted to lists (doh!) - `planned_proejects` should actually be `_planned _projects` - _queued_projects should be truthiness-checked, not ever none

also: - fixed a lot of API usage - added specific Exception types for better error catching in testing - black

lmz · 2023-03-30T20:35:44Z

I was trying to take a look at this by running notebook\Scenario Building Example.ipynb and I'm getting errors when reading the project cards which I'm finding difficult to interpret:

2023-03-30 13:26:54, INFO: Creating Scenario
2023-03-30 13:26:54, WARNING: Base_scenario doesn't contain ['road_net', 'transit_net', 'applied_projects', 'conflicts']
2023-03-30 13:26:54, DEBUG: Finding project cards using glob search: *attribute*.yml in [e:\GitHub\network_wrangler_wsp-sag\examples\stpaul\project_cards](file:///E:/GitHub/network_wrangler_wsp-sag/examples/stpaul/project_cards)
2023-03-30 13:26:54, DEBUG: Reading YAML-Style Project Card [e:\GitHub\network_wrangler_wsp-sag\examples\stpaul\project_cards\1_simple_roadway_attribute_change.yml](file:///E:/GitHub/network_wrangler_wsp-sag/examples/stpaul/project_cards/1_simple_roadway_attribute_change.yml)
2023-03-30 13:26:54, ERROR: Failed Project Card validation: Validation Error
   Project Card File Loc:[e:\GitHub\network_wrangler_wsp-sag\examples\stpaul\project_cards\1_simple_roadway_attribute_change.yml](file:///E:/GitHub/network_wrangler_wsp-sag/examples/stpaul/project_cards/1_simple_roadway_attribute_change.yml)
   Project Card Schema Loc:[E:\GitHub\network_wrangler_wsp-sag\network_wrangler\schemas\project_card.json](file:///E:/GitHub/network_wrangler_wsp-sag/network_wrangler/schemas/project_card.json)
      [{'osm_link_id': ['223371529']}] is valid under each of {'required': ['osm_link_id']}, {'required': ['model_link_id']}, {'required': ['name']}

2023-03-30 13:26:54, DEBUG: Reading YAML-Style Project Card [e:\GitHub\network_wrangler_wsp-sag\examples\stpaul\project_cards\3_multiple_roadway_attribute_change.yml](file:///E:/GitHub/network_wrangler_wsp-sag/examples/stpaul/project_cards/3_multiple_roadway_attribute_change.yml)
2023-03-30 13:26:54, ERROR: Failed Project Card validation: Validation Error
   Project Card File Loc:[e:\GitHub\network_wrangler_wsp-sag\examples\stpaul\project_cards\3_multiple_roadway_attribute_change.yml](file:///E:/GitHub/network_wrangler_wsp-sag/examples/stpaul/project_cards/3_multiple_roadway_attribute_change.yml)
   Project Card Schema Loc:[E:\GitHub\network_wrangler_wsp-sag\network_wrangler\schemas\project_card.json](file:///E:/GitHub/network_wrangler_wsp-sag/network_wrangler/schemas/project_card.json)
      [{'name': ['6th', 'Sixth', 'sixth']}] is valid under each of {'required': ['osm_link_id']}, {'required': ['model_link_id']}, {'required': ['name']}

The error says "... is valid under ..."
Is valid seems like it's good? But it's not? 🤔

e-lo · 2023-04-10T16:28:00Z

The error says "... is valid under ..."
Is valid seems like it's good? But it's not? 🤔

Yeah - this sux and has to do with the project card not the scenario handling...which is fully addressed in a subsequent PR that moves the project card validation to the project card "project" ...but I'll add a quick bridge fix (was trying to not spend too much time on it b/c it will be fixed in #325

e-lo added 2 commits March 7, 2023 15:46

Implements scenario changes from #277

f8379c3

Changes how scenarios works per description in #277 and updates relevant documentation about API changes. Also: - various efficiencies and consistency fixes

Update example notebooks for tiny API changes

d1846a8

e-lo added enhancement New feature or request refactor labels Mar 7, 2023

e-lo added this to the v1.0 milestone Mar 7, 2023

e-lo requested review from lmz and i-am-sijia March 7, 2023 23:55

e-lo self-assigned this Mar 7, 2023

Update to use a deque object

48c708d

So you aren't iterating over a changing list

e-lo added 2 commits March 10, 2023 16:00

Merge branch 'develop' of https://github.com/wsp-sag/network_wrangler …

65fb4f4

…into change_project_apply_handling

Merge updates from develop. Black.

1dc8903

e-lo changed the base branch from master to develop March 13, 2023 14:32

e-lo added 4 commits March 15, 2023 15:44

Merge remote-tracking branch 'origin/develop' into change_project_app…

d4efbcb

…ly_handling

Merge remote-tracking branch 'origin/develop' into change_project_app…

f08abb7

…ly_handling

Merge remote-tracking branch 'origin/develop' into change_project_app…

55b6982

…ly_handling

add a little more documentation

e459a16

i-am-sijia requested changes Mar 17, 2023

View reviewed changes

e-lo and others added 11 commits March 21, 2023 14:53

Update notebook/Scenario Building Example.ipynb

bfa3f17

Co-authored-by: Sijia Wang <[email protected]>

Fix default project_card_list to be [] not None

48ad42b

Also: - Better document Scenario.__init__ arguments - Require minimum variables in base_scenario: applied_projects and conflicts

Merge branch 'change_project_apply_handling' of https://github.com/ws…

297c675

…p-sag/network_wrangler into change_project_apply_handling

Address mapping type --> list and set type needed for isdisjoint

f0dbbd7

Varia fixes from Sijia! (thx!)

cff89fd

- is_disjoint is actually isdisjoint (doh!) and performed on a set object - mapping objects converted to lists (doh!) - `planned_proejects` should actually be `_planned _projects` - _queued_projects should be truthiness-checked, not ever none

Remove universal lower() / fixed validate method ref

e35d844

scenario tests now use fixtures + are consistently formatted

f081e52

also: - fixed a lot of API usage - added specific Exception types for better error catching in testing - black

import copy

f4d9b3a

fix import

b6f0747

Add project card path to the debug statement

086d30a

The suffix includes the period; drop the period for this check

912a254

e-lo linked an issue Apr 10, 2023 that may be closed by this pull request

[Refactor] Rename/define Scenario variables so they make sense when building tiered scenarios #277

Closed

e-lo merged commit 708772f into develop Apr 26, 2023

e-lo deleted the change_project_apply_handling branch April 26, 2023 16:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Scenarios to improve legibility and workflow management. #315

Refactor Scenarios to improve legibility and workflow management. #315

e-lo commented Mar 7, 2023 •

edited by lmz

Loading

e-lo commented Mar 10, 2023

e-lo commented Mar 16, 2023

i-am-sijia left a comment

lmz commented Mar 30, 2023 •

edited

Loading

e-lo commented Apr 10, 2023

Refactor Scenarios to improve legibility and workflow management. #315

Refactor Scenarios to improve legibility and workflow management. #315

Conversation

e-lo commented Mar 7, 2023 • edited by lmz Loading

Updated Definitions

e-lo commented Mar 10, 2023

e-lo commented Mar 16, 2023

i-am-sijia left a comment

Choose a reason for hiding this comment

lmz commented Mar 30, 2023 • edited Loading

e-lo commented Apr 10, 2023

e-lo commented Mar 7, 2023 •

edited by lmz

Loading

lmz commented Mar 30, 2023 •

edited

Loading