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

Vehicle Type Alts Filtering Bug Fix #790

Merged
merged 5 commits into from
Feb 9, 2024
Merged

Conversation

dhensle
Copy link
Contributor

@dhensle dhensle commented Feb 2, 2024

PR for #789

@dhensle dhensle added the Phase 8 Feature planned for inclusion in Phase 8 release label Feb 2, 2024
@dhensle dhensle requested a review from i-am-sijia February 2, 2024 22:20
@jpn--
Copy link
Member

jpn-- commented Feb 5, 2024

When we fix a bug, we need to have a test that proves it's fixed (and so it stays fixed).

@i-am-sijia
Copy link
Contributor

Need to update documentation:

Ok, let's add REQUIRE_DATA_FOR_ALL_ALTS to the model documentation, and update the following blurb to remove "must be included in the file" -

activitysim/docs/models.rst

Lines 448 to 451 in a8e755f

* ``VEHICLE_TYPE_DATA_FILE``: Filename for input vehicle type data. Must have columns ``body_type``, ``fuel_type``, and ``vehicle_year``.
Vehicle ``age`` is computed using the ``FLEET_YEAR`` option. Data for every alternative specified in the ``combinatorial_alts`` option must be included
in the file. Vehicle type data file will be joined to the alternatives and can be used in the utility expressions if ``PROBS_SPEC`` is not provided.
If ``PROBS_SPEC`` is provided, the vehicle type data will be joined after a vehicle type is decided so the data can be used in downstream models.

@jpn--
Copy link
Member

jpn-- commented Feb 8, 2024

I opened a PR #796 to bring ActivitySim's config settings up-to-date with Pydantic 2.0, which should address your most recent test failure (at least in part).

@dhensle
Copy link
Contributor Author

dhensle commented Feb 8, 2024

Thanks! The test was passing locally, so I expect the update should fix the test completely.

@jpn-- jpn-- force-pushed the veh_type_alts_filter_fix branch from bb7f79a to 3521c8d Compare February 8, 2024 22:49
@jpn-- jpn-- merged commit a2ad2a1 into develop Feb 9, 2024
37 checks passed
@jpn-- jpn-- mentioned this pull request Feb 14, 2024
@jpn-- jpn-- deleted the veh_type_alts_filter_fix branch February 20, 2024 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Phase 8 Feature planned for inclusion in Phase 8 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants