Fix preprocessors for detailed fuselage - #1291
Conversation
… when zero passengers in that class
…racefully by the component
| seat_width_first = 25 | ||
|
|
||
| # set defaults for seat pitch | ||
| seat_pitch_economy = 34 |
There was a problem hiding this comment.
I wonder if we should remove the defaults from the variable metadata for these variables that have AircraftType specific defaults.
| 'Aircraft.CrewPayload.Design.NUM_SEATS_ABREAST_ECONOMY not ' | ||
| 'set, assuming default 0.' | ||
| if num_economy_class_pax > 0 and num_seat_abreast_economy <= 0: | ||
| raise ValueError( |
There was a problem hiding this comment.
There is a software engineering practice where we want to have test coverage for every line of code. To cover these, we would need a test that intentionally loads a negative value for each of these, and then verifies that the error is raised with the correct expected text. We haven't been adhering to this so far in early Aviary development, but it would be a good practice to adopt now that we are more mature.
There was a problem hiding this comment.
In general I don't think we have a test for preprocessors at all, so maybe we need to create one, unfortunately it's going to be MASSIVE!!!! What's the best practice? Should there be a separate unit test for every logic branch in preprocessors? Perhaps we can get an AI to help write this!
There was a problem hiding this comment.
This does sound like an ideal low-impact task for an LLM. When I make these, I tend to put a bunch of checks in a single test. I suspect an LLM will make a separate test for each message, and that might be closer to "textbook" unit-testing.
2 participants
Summary
This PR fixes a couple of errors with detailed fuselage component.
The preprocessor logic for fuselages requires passenger numbers and these get_vals were omitted from previous PR.
Reorganize logic for setting of defaults in preprocessors.
Update the fuselage.py detailed component to accept user specifying zero seats abreast when there are zero passengers of that class.
Related Issues
Backwards incompatibilities
None
AI Usage
Disclose any AI usage in this PR, including models used and files affected.