Skip to content

Update Makefile for Darwin #612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Update Makefile for Darwin #612

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f98e16f
Update Makefile
waltsims May 25, 2025
515238f
Find executable file
waltsims May 25, 2025
d08e317
Merge branch 'master' into enhancement/darwin-make-test
waltsims Sep 11, 2025
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
39 changes: 33 additions & 6 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,24 @@ TESTS_DIR = tests
MATLAB_SCRIPT = tests/matlab_test_data_collectors/run_all_collectors.m
KWAVE_MATLAB_PATH = $(abspath ../k-wave) # Get absolute path of k-wave directory

# Define the artifact directory
# Define the artifact directory for collected test values
COLLECTED_VALUES_DIR = $(abspath tests/matlab_test_data_collectors/python_testers/collectedValues)

# Default target
# Platform-specific MATLAB detection
# On macOS, automatically find the latest MATLAB installation
ifeq ($(shell uname), Darwin)
# Find the latest MATLAB version installed in /Applications
# Sort in reverse order and take the first (latest) version
MATLAB_PATH := $(shell ls -d /Applications/MATLAB_R* 2>/dev/null | sort -r | head -n 1)
ifneq ($(MATLAB_PATH),)
MATLAB := $(MATLAB_PATH)/bin/matlab
endif
endif
Comment on lines +14 to +23
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix MATLAB detection: support PATH installs and avoid overriding user overrides.

  • Current Darwin probe won’t detect matlab available via PATH; it only works when MATLAB is an absolute executable path.
  • Also, don’t override a user-provided MATLAB value.

Apply this diff to the Darwin block to prefer user overrides, quote paths, and constrain to .app bundles:

 ifeq ($(shell uname), Darwin)
-    # Find the latest MATLAB version installed in /Applications
-    # Sort in reverse order and take the first (latest) version
-    MATLAB_PATH := $(shell ls -d /Applications/MATLAB_R* 2>/dev/null | sort -r | head -n 1)
-    ifneq ($(MATLAB_PATH),)
-        MATLAB := $(MATLAB_PATH)/bin/matlab
-    endif
+    # Find the latest MATLAB .app bundle in /Applications (if user hasn't set MATLAB)
+    ifeq ($(origin MATLAB), default)
+        MATLAB_PATH := $(shell ls -d /Applications/MATLAB_R*.app 2>/dev/null | sort -r | head -n 1)
+        ifneq ($(MATLAB_PATH),)
+            MATLAB := "$(MATLAB_PATH)"/bin/matlab
+        endif
+    endif
 endif

Additionally (outside this hunk), consider making the top-level defaults override-friendly:

PYTHON ?= python
MATLAB ?= matlab
🤖 Prompt for AI Agents 
In Makefile around lines 14 to 23, the Darwin MATLAB probe overrides
user-provided MATLAB and only detects /Applications/MATLAB_R* directories,
missing PATH-installed matlab; change the logic to first respect any existing
MATLAB variable (do not overwrite if set), set top-level defaults using ?=
(e.g., PYTHON ?= python and MATLAB ?= matlab), and in the Darwin block only run
the .app detection when MATLAB is not already set: search /Applications for
directories matching MATLAB_R*\.app, pick the latest, set MATLAB to the quoted
path to its bin/matlab, and fall back to the PATH-resolved matlab if no .app is
found so PATH installs are supported.


# Default target - runs all examples and tests
all: run-examples test

# Target to run all examples
# Target to run all examples with non-interactive backend
run-examples:
@echo "Running all examples..."
@MPLBACKEND=Agg $(PYTHON) run_examples.py
Expand All @@ -25,18 +36,34 @@ test: $(COLLECTED_VALUES_DIR)
@pytest $(TESTS_DIR)

# Target to run the MATLAB script and create the artifact directory
# Includes fallback behavior if MATLAB is not available
$(COLLECTED_VALUES_DIR):
@echo "Running MATLAB script to collect values..."; \
$(MATLAB) -batch "run('$(MATLAB_SCRIPT)');"; \
# Clean target (optional) - cleans Python caches and collected values
if [ -x "$(MATLAB)" ]; then \
$(MATLAB) -batch "run('$(MATLAB_SCRIPT)');"; \
else \
echo "MATLAB not found. Skipping MATLAB data collection."; \
echo "Some tests may fail if they depend on MATLAB data."; \
echo "To run all tests, you need to:"; \
echo "1. Install MATLAB from https://www.mathworks.com/products/matlab.html"; \
echo "2. Make sure MATLAB is in your PATH or installed in /Applications/MATLAB_R* on macOS"; \
echo "3. Run 'make clean' to clean any partial test data"; \
echo "4. Run 'make test' again"; \
echo ""; \
echo "For now, continuing with available tests..."; \
mkdir -p $(COLLECTED_VALUES_DIR); \
fi

# Clean target - removes all generated files and caches
clean: clean-python clean-collected_values

# Clean Python-specific files
clean-python:
@echo "Cleaning Python cache files..."
@find . -name '*.pyc' -delete
@find . -name '__pycache__' -delete

# Clean collected values directory
# Clean collected test values
clean-collected_values:
@echo "Cleaning collected values directory..."
@rm -rf $(COLLECTED_VALUES_DIR)
Expand Down
Loading