Merge pull request #783 from stan-dev/improve-validation

CmdStan path validation only requires makefile

Author: WardBrian

cmdstanpy/compilation.py

@@ -19,6 +19,7 @@
     cmdstan_path,
     cmdstan_version,
     cmdstan_version_before,
+    stanc_path,
 )
 from cmdstanpy.utils.command import do_command
 from cmdstanpy.utils.filesystem import SanitizedOrTmpFilePath
@@ -351,7 +352,7 @@ def src_info(
     :meth:`CmdStanModel.src_info`, and should not be called directly.
     """
     cmd = (
-        [os.path.join(cmdstan_path(), 'bin', 'stanc' + EXTENSION)]
+        [stanc_path()]
         # handle include-paths, allow-undefined etc
         + compiler_options.compose_stanc(None)
         + ['--info', str(stan_file)]
@@ -518,7 +519,7 @@ def format_stan_file(
 
     try:
         cmd = (
-            [os.path.join(cmdstan_path(), 'bin', 'stanc' + EXTENSION)]
+            [stanc_path()]
             # handle include-paths, allow-undefined etc
             + CompilerOptions(stanc_options=stanc_options).compose_stanc(None)
             + [str(stan_file)]

cmdstanpy/utils/cmdstan.py

@@ -1,6 +1,7 @@
 """
 Utilities for finding and installing CmdStan
 """
+
 import os
 import platform
 import subprocess
@@ -133,15 +134,29 @@ def validate_cmdstan_path(path: str) -> None:
     """
     if not os.path.isdir(path):
         raise ValueError(f'No CmdStan directory, path {path} does not exist.')
-    if not os.path.exists(os.path.join(path, 'bin', 'stanc' + EXTENSION)):
+    if not os.path.exists(os.path.join(path, 'makefile')):
         raise ValueError(
-            f'CmdStan installataion missing binaries in {path}/bin. '
-            'Re-install cmdstan by running command "install_cmdstan '
-            '--overwrite", or Python code "import cmdstanpy; '
-            'cmdstanpy.install_cmdstan(overwrite=True)"'
+            f'CmdStan installataion missing makefile, path {path} is invalid.'
+            ' You may wish to re-install cmdstan by running command '
+            '"install_cmdstan --overwrite", or Python code '
+            '"import cmdstanpy; cmdstanpy.install_cmdstan(overwrite=True)"'
         )
 
 
+def stanc_path() -> str:
+    """
+    Returns the path to the stanc executable in the CmdStan installation.
+    """
+    cmdstan = cmdstan_path()
+    stanc_exe = os.path.join(cmdstan, 'bin', 'stanc' + EXTENSION)
+    if not os.path.exists(stanc_exe):
+        raise ValueError(
+            f'stanc executable not found in CmdStan installation: {cmdstan}.\n'
+            'You may need to re-install or re-build CmdStan.',
+        )
+    return stanc_exe
+
+
 def set_cmdstan_path(path: str) -> None:
     """
     Validate, then set CmdStan directory path.
@@ -200,13 +215,6 @@ def cmdstan_version() -> Optional[Tuple[int, ...]]:
         get_logger().debug("%s", e)
         return None
 
-    if not os.path.exists(makefile):
-        get_logger().info(
-            'CmdStan installation %s missing makefile, cannot get version.',
-            cmdstan_path(),
-        )
-        return None
-
     with open(makefile, 'r') as fd:
         contents = fd.read()
 

test/test_utils.py

@@ -7,11 +7,9 @@
 import os
 import pathlib
 import platform
-import random
 import re
 import shutil
 import stat
-import string
 import tempfile
 from test import check_present, mark_windows_only, raises_nested
 from unittest import mock
@@ -43,6 +41,7 @@
     validate_dir,
     windows_short_path,
 )
+from cmdstanpy.utils.cmdstan import stanc_path
 from cmdstanpy.utils.filesystem import temp_inits, temp_single_json
 
 HERE = os.path.dirname(os.path.abspath(__file__))
@@ -134,6 +133,16 @@ def test_set_path() -> None:
         assert os.path.samefile(install_version, os.environ['CMDSTAN'])
 
 
+@contextlib.contextmanager
+def temporary_cmdstan_path(path: str) -> None:
+    prev = cmdstan_path()
+    try:
+        set_cmdstan_path(path)
+        yield
+    finally:
+        set_cmdstan_path(prev)
+
+
 def test_validate_path() -> None:
     if 'CMDSTAN' in os.environ:
         install_version = os.environ.get('CMDSTAN')
@@ -150,20 +159,18 @@ def test_validate_path() -> None:
     with pytest.raises(ValueError, match='No CmdStan directory'):
         validate_cmdstan_path(path_foo)
 
-    folder_name = ''.join(
-        random.choice(string.ascii_letters) for _ in range(10)
-    )
-    while os.path.exists(folder_name):
-        folder_name = ''.join(
-            random.choice(string.ascii_letters) for _ in range(10)
-        )
-    folder = pathlib.Path(folder_name)
-    folder.mkdir(parents=True)
-    (folder / "makefile").touch()
+    with tempfile.TemporaryDirectory() as tmpdir:
+        folder = pathlib.Path(tmpdir)
+        with pytest.raises(ValueError, match='missing makefile'):
+            validate_cmdstan_path(str(folder.absolute()))
 
-    with pytest.raises(ValueError, match='missing binaries'):
-        validate_cmdstan_path(str(folder.absolute()))
-    shutil.rmtree(folder)
+        (folder / "makefile").touch()
+        with temporary_cmdstan_path(str(folder.absolute())):
+            with pytest.raises(
+                ValueError,
+                match='stanc executable not found in CmdStan installation',
+            ):
+                stanc_path()
 
 
 def test_validate_dir() -> None:
@@ -216,7 +223,6 @@ def test_cmdstan_version(caplog: pytest.LogCaptureFixture) -> None:
         fake_bin.mkdir(parents=True)
         fake_makefile = fake_path / 'makefile'
         fake_makefile.touch()
-        (fake_bin / f'stanc{EXTENSION}').touch()
         with mock.patch.dict("os.environ", CMDSTAN=str(fake_path)):
             assert str(fake_path) == cmdstan_path()
             with open(fake_makefile, 'w') as fd:
@@ -230,10 +236,7 @@ def test_cmdstan_version(caplog: pytest.LogCaptureFixture) -> None:
             check_present(caplog, ('cmdstanpy', 'INFO', expect))
 
             fake_makefile.unlink()
-            expect = (
-                'CmdStan installation {} missing makefile, '
-                'cannot get version.'.format(fake_path)
-            )
+            expect = 'No CmdStan installation found.'
             with caplog.at_level(logging.INFO):
                 cmdstan_version()
             check_present(caplog, ('cmdstanpy', 'INFO', expect))