Fixed Vector bug

Author: dbookstaber

py_ballisticcalc/drag_model.py

@@ -42,6 +42,7 @@ class DragDataPoint:
         Mach: Velocity in Mach units (dimensionless)
         CD: Drag coefficient (dimensionless)
     """
+    
     Mach: float  # Velocity in Mach units
     CD: float  # Drag coefficient
 
@@ -321,6 +322,10 @@ def linear_interpolation(x: Union[List[float], Tuple[float]],
         - Uses binary search for efficient interval location
     """
     assert len(xp) == len(yp), "xp and yp lists must have same length"
+    # Validate xp strictly increasing to prevent zero-division and undefined intervals
+    for i in range(1, len(xp)):
+        if not (xp[i] > xp[i - 1]):
+            raise ValueError("xp must be strictly increasing with no duplicates")
 
     y = []
     for xi in x:

py_ballisticcalc/munition.py

@@ -65,6 +65,7 @@ class SightReticleStep(NamedTuple):
         )
         ```
     """
+
     vertical: Angular
     horizontal: Angular
 
@@ -84,6 +85,7 @@ class SightClicks(NamedTuple):
         clicks = SightClicks(vertical=5.0, horizontal=2.5)
         ```
     """
+
     vertical: float
     horizontal: float
 
@@ -114,6 +116,7 @@ class Sight:
         )
         ```
     """
+
     focal_plane: SightFocalPlane
     scale_factor: Distance
     h_click_size: Angular
@@ -154,7 +157,6 @@ def __init__(self,
             )
             ```
         """
-
         if focal_plane not in get_args(SightFocalPlane):
             raise ValueError("Wrong focal plane")
 
@@ -164,14 +166,14 @@ def __init__(self,
         if (not isinstance(h_click_size, (Angular, float, int))
             or not isinstance(v_click_size, (Angular, float, int))
         ):
-            raise TypeError("type Angular expected for 'h_click_size' and 'v_click_size'")
+            raise TypeError("Angle expected for 'h_click_size' and 'v_click_size'")
 
         self.focal_plane = focal_plane
         self.scale_factor = PreferredUnits.distance(scale_factor or 1)
         self.h_click_size = PreferredUnits.adjustment(h_click_size)
         self.v_click_size = PreferredUnits.adjustment(v_click_size)
         if self.h_click_size.raw_value <= 0 or self.v_click_size.raw_value <= 0:
-            raise TypeError("'h_click_size' and 'v_click_size' have to be positive")
+            raise TypeError("'h_click_size' and 'v_click_size' must be positive")
 
     def _adjust_sfp_reticle_steps(self, target_distance: Union[float, Distance],
                                   magnification: float) -> SightReticleStep:
@@ -200,16 +202,14 @@ def _adjust_sfp_reticle_steps(self, target_distance: Union[float, Distance],
             ```
         """
         assert self.focal_plane == 'SFP', "SFP focal plane required"
-        # adjust reticle scale relative to target distance and magnification
+        _td = PreferredUnits.distance(target_distance)
+        if _td.raw_value <= 0:
+            raise ValueError("target_distance must be positive")
         def get_sfp_step(click_size: Angular):
+            """Calculate SFP reticle step size for a given click size."""
+            scale_ratio = self.scale_factor.raw_value / _td.raw_value
             # Don't need distances conversion because units cancel:
-            return click_size.units(
-                click_size.unit_value
-                * self.scale_factor.raw_value
-                / _td.raw_value
-                * magnification
-            )
-        _td = PreferredUnits.distance(target_distance)
+            return click_size.units(click_size.unit_value * scale_ratio * magnification)
         _h_step = get_sfp_step(self.h_click_size)
         _v_step = get_sfp_step(self.v_click_size)
         return SightReticleStep(_h_step, _v_step)
@@ -251,6 +251,8 @@ def get_adjustment(self, target_distance: Distance,
             print(f"Adjust: {clicks.vertical} up, {clicks.horizontal} right")
             ```
         """
+        if magnification <= 0:
+            raise ValueError("magnification must be positive")
         if self.focal_plane == 'SFP':
             steps = self._adjust_sfp_reticle_steps(target_distance, magnification)
             return SightClicks(
@@ -263,8 +265,7 @@ def get_adjustment(self, target_distance: Distance,
                 windage_adj.raw_value / self.h_click_size.raw_value
             )
         if self.focal_plane == 'LWIR':
-            # adjust clicks to magnification
-            return SightClicks(
+            return SightClicks(  # adjust clicks to magnification
                 drop_adj.raw_value / (self.v_click_size.raw_value / magnification),
                 windage_adj.raw_value / (self.h_click_size.raw_value / magnification)
             )
@@ -328,6 +329,7 @@ class Weapon:
         )
         ```
     """
+    
     sight_height: Distance
     twist: Distance
     zero_elevation: Angular
@@ -510,6 +512,8 @@ def calc_powder_sens(self, other_velocity: Union[float, Velocity],
         v1 = PreferredUnits.velocity(other_velocity) >> Velocity.MPS
         t1 = PreferredUnits.temperature(other_temperature) >> Temperature.Celsius
 
+        if v0 <= 0 or v1 <= 0:
+            raise ValueError("calc_powder_sens requires positive muzzle velocities")
         v_delta = math.fabs(v0 - v1)
         t_delta = math.fabs(t0 - t1)
         v_lower = v1 if v1 < v0 else v0

py_ballisticcalc/vector.py

@@ -85,15 +85,10 @@ class Vector(NamedTuple):
     z: float
 
     def magnitude(self) -> float:
-        """Calculate the Euclidean magnitude (length) of the vector.
-        
-        Computes the magnitude using the optimized math.hypot() function which
-        provides better numerical accuracy and avoids overflow/underflow issues
-        compared to naive sqrt(x²+y²+z²) implementation.
+        """Calculate the Euclidean norm (length) of the vector.
         
         Returns:
             The magnitude (length) of the vector as a non-negative float.
-            Returns 0.0 for zero vectors.
             
         Examples:
             ```python
@@ -103,7 +98,7 @@ def magnitude(self) -> float:
             
             # Velocity magnitude (speed)
             velocity = Vector(800.0, 100.0, 50.0)
-            speed = velocity.magnitude()  # ~806.5 m/s
+            speed = velocity.magnitude()  # ~806.5
             
             # Distance calculation
             position = Vector(100.0, 50.0, 25.0)
@@ -457,33 +452,6 @@ def __sub__(self, other: Vector) -> Vector:  # type: ignore[override]
         """
         return self.subtract(other)
 
-    def __rsub__(self, other: Vector) -> Vector:  # type: ignore[override]
-        """Right subtraction operator for vector subtraction.
-        
-        Enables vector subtraction when this vector is on the right side of
-        the - operator. Note that vector subtraction is NOT commutative,
-        so this performs other - self, not self - other.
-        
-        Args:
-            other: The Vector instance to subtract this vector from.
-            
-        Returns:
-            New Vector instance representing the difference (other - self).
-            
-        Examples:
-            ```python
-            v1 = Vector(10.0, 5.0, 0.0)
-            v2 = Vector(3.0, 2.0, 1.0)
-            # When using v1 - v2, if v2.__rsub__ is called:
-            result = v1 - v2  # Actually calls v2.__rsub__(v1) = v1 - v2
-            ```
-            
-        Note:
-            The implementation delegates to subtract() which computes self - other.
-            This may need review for correct right-subtraction semantics.
-        """
-        return self.subtract(other)
-
     def __isub__(self, other: Vector) -> Vector:  # type: ignore[override]
         """In-place subtraction operator for vector subtraction.
         

tests/test_vector.py

@@ -69,11 +69,7 @@ def test_vector_right_ops_and_inplace_aliases(self):
         sum2 = v1.__iadd__(Vector(0.0, -1.0, 1.0))
         assert sum2 == Vector(1.0, 1.0, 4.0)
 
-        # __rsub__ and __isub__ paths
-        # Current implementation delegates to subtract(self, other) => self - other
-        # so __rsub__ behaves like regular __sub__ with swapped args not applied
-        diff1 = v1.__rsub__(Vector(0.0, 0.0, 0.0))
-        assert diff1 == Vector(1.0, 2.0, 3.0)
+        # __isub__ paths
         diff2 = v1.__isub__(Vector(1.0, 1.0, 1.0))
         assert diff2 == Vector(0.0, 1.0, 2.0)