Issue 1: Mutable Default Argument in add_vehicle_break
File: python/cuopt/routing.py
Method: add_vehicle_break()
Description
The add_vehicle_break method uses a mutable default argument locations=cudf.Series(). In Python, default argument values are evaluated only once at function definition, not each time the function is called. This can lead to unexpected behavior where the same Series object is shared across multiple calls.
Current Code
def add_vehicle_break(
self, vehicle_id, earliest, latest, duration, locations=cudf.Series()
):
validate_range(vehicle_id, "vehicle id", 0, self.get_fleet_size())
if len(locations) > 0:
validate_range(locations, "break locations", 0, self.get_num_locations())
super().add_vehicle_break(vehicle_id, earliest, latest, duration, locations)Expected Behavior
Use None as default and create a new Series inside the function to avoid shared state issues.
Proposed Fix
def add_vehicle_break(
self, vehicle_id, earliest, latest, duration, locations=None
):
validate_range(vehicle_id, "vehicle id", 0, self.get_fleet_size())
if locations is None:
locations = cudf.Series()
if len(locations) > 0:
validate_range(locations, "break locations", 0, self.get_num_locations())
super().add_vehicle_break(vehicle_id, earliest, latest, duration, locations)Impact
- Prevents unintended shared state across function calls
- Follows Python best practices (PEP 8)
- Improves code reliability and maintainability
Issue 2: Documentation Examples Contain Errors
Multiple documentation examples contain copy-paste errors and typos that may confuse users or cause runtime errors when copied directly.
2.1 add_cost_matrix Example - Wrong Variable Assignment
Current (incorrect):
>>> cost_mat_car = cudf.DataFrame(cost_mat_bikes) # ❌ Uses bikes matrix for car
>>> cost_mat_car
0 1 2 3
0 0 1 5 2 # Shows bike data, not car data
1 2 0 7 4
2 1 5 0 9
3 5 6 2 0
Expected (correct):
>>> cost_mat_car = cudf.DataFrame([
... [0, 1, 2, 1],
... [1, 0, 3, 2],
... [1, 2, 0, 3],
... [1, 3, 9, 0]
... ])
>>> cost_mat_car
0 1 2 3
0 0 1 2 1 # ✅ Correct car matrix
1 1 0 3 2
2 1 2 0 3
3 1 3 9 0
2.2 set_vehicle_types Example - Typo and Undefined Variable
Current (incorrect):
>>> vehicle_tpes = [0, 1, 1, 0, 0] # ❌ Typo: "tpes" instead of "types"
>>> data_model.set_vehicle_types(cudf.Series(vehicle_types)) # ❌ vehicle_types undefined
Expected (correct):
>>> vehicle_types = [0, 1, 1, 0, 0] # ✅ Correct variable name
>>> data_model.set_vehicle_types(cudf.Series(vehicle_types)) # ✅ Uses defined variable
Impact
- Prevents user confusion when copying examples
- Avoids runtime errors from undefined variables
- Improves documentation reliability and professional appearance
- Better developer experience for new users
Environment
- cuOpt Version: Latest (as shown in code, copyright 2021-2026)
- Python Version: 3.x
- Related Files:
python/cuopt/routing.py
Additional Notes:
- These issues were found during code review of the public repository
Issue 1: Mutable Default Argument in
add_vehicle_breakFile:
python/cuopt/routing.pyMethod:
add_vehicle_break()Description
The
add_vehicle_breakmethod uses a mutable default argumentlocations=cudf.Series(). In Python, default argument values are evaluated only once at function definition, not each time the function is called. This can lead to unexpected behavior where the same Series object is shared across multiple calls.Current Code
Expected Behavior
Use
Noneas default and create a new Series inside the function to avoid shared state issues.Proposed Fix
Impact
Issue 2: Documentation Examples Contain Errors
Multiple documentation examples contain copy-paste errors and typos that may confuse users or cause runtime errors when copied directly.
2.1
add_cost_matrixExample - Wrong Variable AssignmentCurrent (incorrect):
Expected (correct):
2.2
set_vehicle_typesExample - Typo and Undefined VariableCurrent (incorrect):
Expected (correct):
Impact
Environment
python/cuopt/routing.pyAdditional Notes: