Avoid divide-by-zero when there is no packing list - #403
Conversation
lift_report_to_blocks and ship_report_to_blocks computed the success percentage as num_success_pallets / total_pallets * 100. When a ship runs with no packing list, total_pallets is 0 (or missing, so _safely_access returns None), which raised ZeroDivisionError / TypeError and aborted the ship. Guard the division: when total_pallets is falsy, report 100 (no pallets means nothing failed). Adds no-packing-list tests for both the lift and ship report paths. Fixes #391 Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a runtime failure in Slack report generation when shipping/lifting runs with no packing list (i.e., total_pallets == 0), preventing ZeroDivisionError/TypeError and allowing the scheduled ship→lift workflow to complete (Fixes #391).
Changes:
- Guard success-percentage division in
lift_report_to_blocksandship_report_to_blockswhentotal_palletsis falsy. - Add unit tests covering ship/lift reports with
total_pallets = 0to ensure the guarded path doesn’t raise.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/forklift/slack.py |
Adds a total_pallets guard so success percentage calculation won’t divide by zero when no packing list exists. |
tests/test_slack.py |
Adds tests to exercise ship/lift Slack report generation when total_pallets is 0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for the contribution @arpitjain099! Do you use this project? If so, I'd love to hear about it. |
|
@stdavis Thanks for the quick review and merge! I don't use forklift directly - spotted the issue while browsing open bugs and the zero-division traceback looked like a clean fix. Cool project honestly, the pallet/warehouse metaphor for ETL is fun. |
|
Well thanks for the random contribution! |
Fixes #391.
lift_report_to_blocksandship_report_to_blockscompute the success percentage asnum_success_pallets / total_pallets * 100. When a ship runs with no packing list,total_palletsis0(or missing, in which case_safely_accessreturnsNone), so this raisesZeroDivisionError/TypeErrorand the ship fails - the traceback in the issue points atslackline 140.Both report paths now guard the division: when
total_palletsis falsy the percentage is reported as100(no pallets means nothing failed, so the existing:100:path renders "0 of 0 pallets ran successfully"). I went with 100 rather than 0 since an empty run has no failures; happy to switch to 0 or a "no pallets" message if you prefer.Added
test_ship_with_no_packing_listandtest_lift_with_no_packing_list. I could not run the suite locally becauseforklift.modelsimportsarcpy(ArcGIS Pro, not installable outside an ArcGIS environment), so I verified the percentage logic standalone (total=0andtotal=Noneboth yield100; normal ratios unchanged) andpy_compiled the changed files. The new tests follow the existingtest_slack.pystructure and exercise the guarded path.