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

Support road pricing configuration #63

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Support road pricing configuration #63

wants to merge 17 commits into from

Conversation

gac55
Copy link
Collaborator

@gac55 gac55 commented Aug 22, 2022

Addition of RP params

Closes #62

@gac55 gac55 requested a review from fredshone August 22, 2022 12:59
@mfitz mfitz self-requested a review August 22, 2022 13:02
Copy link
Contributor

@mfitz mfitz left a comment

Choose a reason for hiding this comment

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

We should add one or more tests for this in here, I think.

@gac55
Copy link
Collaborator Author

gac55 commented Aug 22, 2022

We should add one or more tests for this in here, I think.

The existing tests focus on checking get_paramsets_search for scoringParameters and there are no example tests for other additional configurations, for example, Hermes or transitRouter. What would you suggest we assert/test @mfitz ?

@mfitz
Copy link
Contributor

mfitz commented Aug 22, 2022

We should add one or more tests for this in here, I think.

The existing tests focus on checking get_paramsets_search for scoringParameters and there are no example tests for other additional configurations, for example, Hermes or transitRouter. What would you suggest we assert/test @mfitz ?

Add a (minimal) XML config file to the test data that has an RP module in it, and write a test that reads that config file in. You would query the dict that results from parsing the config file and make assertions that it has key/value pairs that match the RP config in the XML file.

@mfitz
Copy link
Contributor

mfitz commented Feb 6, 2023

Does this PR want closing without merging now @gac55 @fredshone ?

@fredshone
Copy link
Contributor

Can't find any example of us using this in config. I think we run via controller?

Found this but is inconsistent with PR.

Last resort -> am restoring an old output TII config to see if i can find there. If not killing this PR.

@fredshone
Copy link
Contributor

@mfitz v simple tests added. I also snuck in keys for another module than is being tested.

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

Successfully merging this pull request may close these issues.

Roadpricing
5 participants