wsp-sag / network_wrangler

A Python Library for Managing Travel Model Network Scenarios
https://wsp-sag.github.io/network_wrangler/
Apache License 2.0
13 stars 3 forks source link

Implement functionality for project card to dictate how to handle sco… #375

Closed e-lo closed 6 days ago

e-lo commented 3 weeks ago

Fixes #373 as described in that issue. Associated required issue: https://github.com/network-wrangler/projectcard/issues/32

e-lo commented 3 weeks ago

@yueshuaing -

e-lo commented 3 weeks ago

TODO:

e-lo commented 3 weeks ago

@yueshuaing and @i-am-sijia please confirm this is implemented as desired

e-lo commented 1 week ago

@yueshuaing and @i-am-sijia please confirm if this approach looks GTG and i can resolve conflicts and merge.

i-am-sijia commented 1 week ago

@yueshuaing and @i-am-sijia please confirm if this approach looks GTG and i can resolve conflicts and merge.

Yue tested it and it did not work, I was referring to it in my email as the two managed lane project cards case. @yueshuaing can you please provide error message?

e-lo commented 1 week ago

I'll see if resolving the conflicting files will help...

yueshuaing commented 1 week ago

@yueshuaing and @i-am-sijia please confirm if this approach looks GTG and i can resolve conflicts and merge.

Yue tested it and it did not work, I was referring to it in my email as the two managed lane project cards case. @yueshuaing can you please provide error message?

The error message is similar to last time. Seems that the validation step requires a specific datatype dictionary or IndivScopedPropertySetItem, but the input is ScopedPropertySetList

validation error for _resolve_conflicting_scopes
1
Input should be a valid dictionary or instance of IndivScopedPropertySetItem [type=model_type, input_value=ScopedPropertySetList(root=[ScopedPropertySetList(...icts=False, change=-1)]), input_type=ScopedPropertySetList]
i-am-sijia commented 1 week ago

Here is an example in Rachel's workflow:

There is a base year MnPass managed lane project card add ML for link 35474: https://github.com/Metropolitan-Council/project_card_registry/blob/378196713303be5568d2340812f90c435b232e6c/projects/BaseMnPASS/394_EB_MnPASS.yml#L39

There is a future year MnPass managed lane project card that expands link 35474 and its ML: https://github.com/Metropolitan-Council/project_card_registry/blob/Test1_converted_project_cards/projects/NoBuild_MnPASS2023/394_louisiana_ave_option2.yml

Both of these project cards will be applied to link 35474 in future year scenario. Would it be an issue? We will test it out.

@e-lo I thought this PR is to address use cases like this. Maybe I'm wrong? @yueshuaing will test applying these two projects with this PR.

e-lo commented 1 week ago

Found the issue using the test – I didn't appropriately handle the case where there were existing scoped link entries but where none were conflicting and they were all matching and implementing a change (not set). Working to fix now.

e-lo commented 6 days ago

I think this can be safely merged at this point.

yueshuaing commented 5 days ago

I've tested the two project cards @i-am-sijia mentioned above, confirming it is working fine now!