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

Remove hard-coded area-based bounds from Purpose #459

Merged
merged 4 commits into from
Sep 22, 2022
Merged

Conversation

zptro
Copy link
Collaborator

@zptro zptro commented Sep 2, 2022

Introduce more general sub-bounds that can define separate parameters for arbitrary number of sub-regions instead of two.

Introduce more general sub-bounds that can define separate parameters
for arbitrary number of sub-regions instead of two
@zptro zptro requested a review from johpiip September 2, 2022 06:16
johpiip
johpiip previously approved these changes Sep 13, 2022
Copy link
Contributor

@johpiip johpiip left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I understand what you are going for but the code is too complex for me to review. Tests are passing so I guess that is enough. I added one comment on the purpose_areas variable!

Scripts/parameters/zone.py Show resolved Hide resolved
@zptro zptro requested a review from johpiip September 19, 2022 12:22
@zptro
Copy link
Collaborator Author

zptro commented Sep 22, 2022

Everything seems good here, but I am not authorized to merge, @johpiip.

@johpiip
Copy link
Contributor

johpiip commented Sep 22, 2022

Everything seems good here, but I am not authorized to merge, @johpiip.

Sorryyy, I did not realize! 😢

@johpiip johpiip merged commit ac46d6e into olusanya Sep 22, 2022
@johpiip johpiip deleted the feat/sub-bounds branch September 22, 2022 07:19
johpiip added a commit that referenced this pull request Sep 25, 2022
* Speed up histogram incrementation by changing lookup method (#452)

* Fix floating-point number error (#455)

* Fix bicycle trips CBA (#463)

Fix incorrect linking of bicycle trips in CBA Excel sheet that causes travel time saving of leisure trips being calculated based on work trips data.

* Remove hard-coded area-based bounds from `Purpose` (#459)

* Remove hard-coded area-based bounds from `Purpose`

Introduce more general sub-bounds that can define separate parameters
for arbitrary number of sub-regions instead of two

* Generalize transit trips per month accordingly

* Add helpful comment

Co-authored-by: Jens West <jens.west@hsl.fi>
Co-authored-by: Sami Mäkinen <47142755+samakinen@users.noreply.github.com>
Co-authored-by: Jens West <jens.west@traficom.fi>
johpiip added a commit that referenced this pull request Sep 25, 2022
* Fix/noise zones (#450)

* Print nodes to better debug errors

* Print 5 meter noise buffers for negative values

* Set `cross_traffic` minimum value to 0.01

* Add error message

* Revert zone width change

Co-authored-by: Jens West <jens.west@hsl.fi>

* Update `HELMET_VERSION` (#451)

* Speed up histogram incrementation by changing lookup method (#452)

* Fix floating-point number error (#455)

* Fix bicycle trips CBA (#463)

Fix incorrect linking of bicycle trips in CBA Excel sheet that causes travel time saving of leisure trips being calculated based on work trips data.

* Remove hard-coded area-based bounds from `Purpose` (#459)

* Remove hard-coded area-based bounds from `Purpose`

Introduce more general sub-bounds that can define separate parameters
for arbitrary number of sub-regions instead of two

* Generalize transit trips per month accordingly

* Add helpful comment

Co-authored-by: Jens West <jens.west@hsl.fi>
Co-authored-by: Sami Mäkinen <47142755+samakinen@users.noreply.github.com>
Co-authored-by: Jens West <jens.west@traficom.fi>
@johpiip johpiip added this to the v4.1.2 milestone Jan 23, 2023
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.

2 participants