Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/numta/api/momentum_indicators.py
Original file line number Diff line number Diff line change
Expand Up @@ -2534,11 +2534,12 @@ def MFI(high: Union[np.ndarray, list],
if timeperiod < 2:
raise ValueError("timeperiod must be >= 2")

# Convert to numpy arrays
high = np.asarray(high, dtype=np.float64)
low = np.asarray(low, dtype=np.float64)
close = np.asarray(close, dtype=np.float64)
volume = np.asarray(volume, dtype=np.float64)
# Convert to numpy arrays with float64 dtype and ensure contiguous memory layout
# This prevents potential segfaults in numba JIT-compiled functions
high = np.ascontiguousarray(np.asarray(high, dtype=np.float64))
low = np.ascontiguousarray(np.asarray(low, dtype=np.float64))
close = np.ascontiguousarray(np.asarray(close, dtype=np.float64))
volume = np.ascontiguousarray(np.asarray(volume, dtype=np.float64))
Comment on lines +2537 to +2542
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The np.ascontiguousarray wrapper is only applied to the MFI function, but not to other similar volume-based indicators like AD, OBV, and ADOSC in src/numta/api/volume_indicators.py, which also use numba JIT compilation and accept the same types of inputs.

If non-contiguous arrays cause segfaults in MFI's numba functions, the same issue could potentially affect other indicators. Consider either:

  1. Applying this fix consistently to all similar numba-compiled functions that accept multiple array inputs, or
  2. Documenting why MFI specifically requires contiguous arrays while others don't

This ensures consistency and prevents similar issues from occurring with other indicators.

Copilot uses AI. Check for mistakes.
Comment on lines +2539 to +2542
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double wrapping np.ascontiguousarray(np.asarray(...)) is inefficient. Consider using np.ascontiguousarray directly with the dtype parameter instead:

high = np.ascontiguousarray(high, dtype=np.float64)
low = np.ascontiguousarray(low, dtype=np.float64)
close = np.ascontiguousarray(close, dtype=np.float64)
volume = np.ascontiguousarray(volume, dtype=np.float64)

This avoids creating an intermediate array when the input is not already a numpy array, improving performance while achieving the same result.

Suggested change
high = np.ascontiguousarray(np.asarray(high, dtype=np.float64))
low = np.ascontiguousarray(np.asarray(low, dtype=np.float64))
close = np.ascontiguousarray(np.asarray(close, dtype=np.float64))
volume = np.ascontiguousarray(np.asarray(volume, dtype=np.float64))
high = np.ascontiguousarray(high, dtype=np.float64)
low = np.ascontiguousarray(low, dtype=np.float64)
close = np.ascontiguousarray(close, dtype=np.float64)
volume = np.ascontiguousarray(volume, dtype=np.float64)

Copilot uses AI. Check for mistakes.

n = len(high)
if len(low) != n or len(close) != n or len(volume) != n:
Expand Down
9 changes: 8 additions & 1 deletion tests/test_pandas_ext_comprehensive.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,14 @@ def test_ad_calculation(self, sample_df):

def test_mfi_calculation(self, sample_df):
"""Test MFI calculation."""
result = sample_df.ta.mfi(timeperiod=14)
# Ensure OHLCV data has correct dtypes to prevent numba JIT issues
df = sample_df.copy()
df['high'] = df['high'].astype(np.float64)
df['low'] = df['low'].astype(np.float64)
df['close'] = df['close'].astype(np.float64)
df['volume'] = df['volume'].astype(np.float64)
Comment on lines +362 to +365
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explicit dtype casting here is redundant. The pandas accessor's _get_column method (line 119 in src/numta/pandas_ext.py) already converts DataFrame column values to np.float64 using .values.astype(np.float64). Additionally, the MFI function itself now ensures correct dtype conversion with np.asarray(..., dtype=np.float64).

This redundant casting adds unnecessary overhead and makes the test inconsistent with other volume indicator tests (test_obv_calculation and test_ad_calculation) which don't perform explicit casting.

Suggested change
df['high'] = df['high'].astype(np.float64)
df['low'] = df['low'].astype(np.float64)
df['close'] = df['close'].astype(np.float64)
df['volume'] = df['volume'].astype(np.float64)

Copilot uses AI. Check for mistakes.

result = df.ta.mfi(timeperiod=14)
assert isinstance(result, pd.Series)

# MFI should be bounded between 0 and 100
Expand Down
Loading