Merge pull request #472 from stan-dev/issue/401-cmdstan-version-at

logic fix for cmdstan_version_at, get_latest_cmdstan

Author: mitzimorris

cmdstanpy/cmdstan_args.py

@@ -13,7 +13,7 @@
 from cmdstanpy import _TMPDIR
 from cmdstanpy.utils import (
     cmdstan_path,
-    cmdstan_version_at,
+    cmdstan_version_before,
     create_named_text_file,
     get_logger,
     read_metric,
@@ -818,7 +818,7 @@ def validate(self) -> None:
                     'Argument "sig_figs" must be an integer between 1 and 18,'
                     ' found {}'.format(self.sig_figs)
                 )
-            if not cmdstan_version_at(2, 25):
+            if cmdstan_version_before(2, 25):
                 self.sig_figs = None
                 get_logger().warning(
                     'Argument "sig_figs" invalid for CmdStan versions < 2.25, '

cmdstanpy/stanfit.py

@@ -44,7 +44,7 @@
     EXTENSION,
     check_sampler_csv,
     cmdstan_path,
-    cmdstan_version_at,
+    cmdstan_version_before,
     create_named_text_file,
     do_command,
     flatten_chains,
@@ -879,7 +879,7 @@ def summary(
             dir=_TMPDIR, prefix=tmp_csv_file, suffix='.csv', name_only=True
         )
         csv_str = '--csv_filename={}'.format(tmp_csv_path)
-        if not cmdstan_version_at(2, 24):
+        if cmdstan_version_before(2, 24):
             csv_str = '--csv_file={}'.format(tmp_csv_path)
         cmd = [
             cmd_path,

cmdstanpy/utils.py

@@ -84,38 +84,43 @@ def get_latest_cmdstan(cmdstan_dir: str) -> Optional[str]:
     """
     Given a valid directory path, find all installed CmdStan versions
     and return highest (i.e., latest) version number.
-    Assumes directory populated via `install_cmdstan`.
+
+    Assumes directory consists of CmdStan releases, created by
+    function `install_cmdstan`, and therefore dirnames have format
+    "cmdstan-<maj>.<min>.<patch>" or "cmdstan-<maj>.<min>.<patch>-rc<num>",
+    which is CmdStan release practice as of v 2.24.
     """
     versions = [
-        ''.join(name.split('-')[1:])  # name may contain '-rc'
+        name[8:]
         for name in os.listdir(cmdstan_dir)
         if os.path.isdir(os.path.join(cmdstan_dir, name))
         and name.startswith('cmdstan-')
         and name[8].isdigit()
+        and len(name[8:].split('.')) == 3
     ]
-    # munge rc for sort, e.g. 2.25.0-rc1 -> 2.25.0.-99
+    if len(versions) == 0:
+        return None
+    # munge rc for sort, e.g. 2.25.0-rc1 -> 2.25.-99
     for i in range(len(versions)):  # # pylint: disable=C0200
-        tmp = versions[i].split('rc')
-        if len(tmp) == 1:
-            versions[i] = '.'.join([tmp[0], '0'])
-        else:
-            rc_sortable = str(int(tmp[1]) - 100)
-            versions[i] = '.'.join([tmp[0], rc_sortable])
+        if '-rc' in versions[i]:
+            comps = versions[i].split('-rc')
+            mmp = comps[0].split('.')
+            rc_num = comps[1]
+            patch = str(int(rc_num) - 100)
+            versions[i] = '.'.join([mmp[0], mmp[1], patch])
 
     versions.sort(key=lambda s: list(map(int, s.split('.'))))
-    if len(versions) == 0:
-        return None
-    latest = 'cmdstan-{}'.format(versions[len(versions) - 1])
+    latest = versions[len(versions) - 1]
 
-    # unmunge
-    tmp = latest.split('.')
-    prefix = '.'.join(tmp[0:3])
-    if int(tmp[3]) == 0:
-        latest = prefix
-    else:
-        tmp[3] = 'rc' + str(int(tmp[3]) + 100)
-        latest = '-'.join([prefix, tmp[3]])
-    return latest
+    # unmunge as needed
+    mmp = latest.split('.')
+    if int(mmp[2]) < 0:
+        print("here")
+        rc_num = str(int(mmp[2]) + 100)
+        mmp[2] = "0-rc" + rc_num
+        latest = '.'.join(mmp)
+
+    return 'cmdstan-' + latest
 
 
 def validate_cmdstan_path(path: str) -> None:
@@ -177,48 +182,75 @@ def cmdstan_path() -> str:
     return cmdstan
 
 
-def cmdstan_version_at(maj: int, min: int) -> bool:
+def cmdstan_version() -> Optional[Tuple[int, ...]]:
     """
-    Check that CmdStan version is at or above Maj.min version.
-    Parses version string out of CmdStan makefile in CmdStan path dir.
+    Parses version string out of CmdStan makefile variable CMDSTAN_VERSION,
+    returns Tuple(Major, minor).
 
-    :param maj: Major version number
-    :param min: Minor version number
-
-    :return: True if version at or above, else False
+    If CmdStan installation is not found or cannot parse version from makefile
+    logs warning and returns None.  Lenient behavoir required for CI tests,
+    per comment:
+    https://github.com/stan-dev/cmdstanpy/pull/321#issuecomment-733817554
     """
-    # pylint:disable=bare-except
     try:
-        path = cmdstan_path()
-        makefile = os.path.join(path, 'makefile')
-        if not os.path.exists(makefile):
-            raise ValueError(
-                'CmdStan installation {}: missing makefile'.format(path)
-            )
-        version = None
-        with open(makefile, 'r') as fd:
-            contents = fd.read()
-            start_idx = contents.find('CMDSTAN_VERSION := ') + len(
-                'CMDSTAN_VERSION := '
-            )
-            end_idx = contents.find('\n', start_idx)
-            version = contents[start_idx:end_idx]
-        if version is None:
-            raise ValueError(
-                'Cannot parse version from makefile: {}'.format(makefile)
-            )
-        splits = version.split('.')
-        if len(splits) < 2:
-            raise ValueError(
-                'Cannot parse version from makefile: {}'.format(makefile)
-            )
-        cur_maj = int(splits[0])
-        cur_min = int(splits[1])
+        makefile = os.path.join(cmdstan_path(), 'makefile')
+    except ValueError:
+        get_logger().info('No CmdStan installation found.')
+        return None
 
-        if cur_maj > maj or (cur_maj == maj and cur_min >= min):
-            return True
-    except:  # noqa
-        pass
+    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()
+
+    start_idx = contents.find('CMDSTAN_VERSION := ')
+    if start_idx < 0:
+        get_logger().info(
+            'Cannot parse version from makefile: %s.',
+            makefile,
+        )
+        return None
+
+    start_idx += len('CMDSTAN_VERSION := ')
+    end_idx = contents.find('\n', start_idx)
+
+    version = contents[start_idx:end_idx]
+    splits = version.split('.')
+    if len(splits) != 3:
+        get_logger().info(
+            'Cannot parse version, expected "<major>.<minor>.<patch>", '
+            'found: "%s".',
+            version,
+        )
+        return None
+    return tuple(int(x) for x in splits[0:2])
+
+
+def cmdstan_version_before(major: int, minor: int) -> bool:
+    """
+    Check that CmdStan version is less than Major.minor version.
+
+    :param major: Major version number
+    :param minor: Minor version number
+
+
+    :return: True if version at or above major.minor, else False.
+    """
+    cur_version = cmdstan_version()
+    if cur_version is None:
+        get_logger().info(
+            'Cannot determine whether version is before %d.%d.', major, minor
+        )
+        return False
+    if cur_version[0] < major or (
+        cur_version[0] == major and cur_version[1] < minor
+    ):
+        return True
     return False
 
 

test/test_cmdstan_args.py

@@ -17,7 +17,7 @@
     SamplerArgs,
     VariationalArgs,
 )
-from cmdstanpy.utils import cmdstan_version_at
+from cmdstanpy.utils import cmdstan_version_before
 
 HERE = os.path.dirname(os.path.abspath(__file__))
 DATAFILES_PATH = os.path.join(HERE, 'data')
@@ -576,7 +576,7 @@ def test_args_bad(self):
     def test_args_sig_figs(self):
         sampler_args = SamplerArgs()
         cmdstan_path()  # sets os.environ['CMDSTAN']
-        if not cmdstan_version_at(2, 25):
+        if cmdstan_version_before(2, 25):
             with LogCapture() as log:
                 logging.getLogger()
                 CmdStanArgs(
@@ -588,7 +588,7 @@ def test_args_sig_figs(self):
                 )
             expect = (
                 'Argument "sig_figs" invalid for CmdStan versions < 2.25, '
-                'using verson {} in directory {}'
+                'using version {} in directory {}'
             ).format(
                 os.path.basename(cmdstan_path()),
                 os.path.dirname(cmdstan_path()),

test/test_sample.py

@@ -27,7 +27,7 @@
 from cmdstanpy.cmdstan_args import CmdStanArgs, Method, SamplerArgs
 from cmdstanpy.model import CmdStanModel
 from cmdstanpy.stanfit import CmdStanMCMC, RunSet, from_csv
-from cmdstanpy.utils import EXTENSION, cmdstan_version_at
+from cmdstanpy.utils import EXTENSION, cmdstan_version_before
 
 HERE = os.path.dirname(os.path.abspath(__file__))
 DATAFILES_PATH = os.path.join(HERE, 'data')
@@ -557,7 +557,7 @@ def test_bernoulli_path_with_space(self):
         )
 
     def test_index_bounds_error(self):
-        if cmdstan_version_at(2, 25) or cmdstan_version_at(2, 26):
+        if not cmdstan_version_before(2, 27):
             oob_stan = os.path.join(DATAFILES_PATH, 'out_of_bounds.stan')
             oob_model = CmdStanModel(stan_file=oob_stan)
             with self.assertRaises(RuntimeError):
@@ -1496,7 +1496,7 @@ def test_validate(self):
         self.assertEqual(bern_fit.metric_type, 'diag_e')
 
     def test_validate_sample_sig_figs(self, stanfile='bernoulli.stan'):
-        if cmdstan_version_at(2, 25):
+        if not cmdstan_version_before(2, 25):
             stan = os.path.join(DATAFILES_PATH, stanfile)
             bern_model = CmdStanModel(stan_file=stan)
 
@@ -1568,7 +1568,7 @@ def test_validate_summary_sig_figs(self):
         beta1_default = format(sum_default.iloc[1, 0], '.18g')
         self.assertTrue(beta1_default.startswith('1.3'))
 
-        if cmdstan_version_at(2, 25):
+        if not cmdstan_version_before(2, 25):
             sum_17 = fit.summary(sig_figs=17)
             beta1_17 = format(sum_17.iloc[1, 0], '.18g')
             self.assertTrue(beta1_17.startswith('1.345767078273'))

test/test_utils.py

@@ -13,6 +13,7 @@
 import string
 import tempfile
 import unittest
+from pathlib import Path
 
 import numpy as np
 import pandas as pd
@@ -28,7 +29,8 @@
     TemporaryCopiedFile,
     check_sampler_csv,
     cmdstan_path,
-    cmdstan_version_at,
+    cmdstan_version,
+    cmdstan_version_before,
     do_command,
     flatten_chains,
     get_latest_cmdstan,
@@ -202,9 +204,46 @@ def test_munge_cmdstan_versions(self):
             os.makedirs(os.path.join(tdir, 'cmdstan-2.22.0'))
             self.assertEqual(get_latest_cmdstan(tdir), 'cmdstan-2.22.0')
 
-    def test_cmdstan_version_at(self):
+    def test_cmdstan_version_before(self):
         cmdstan_path()  # sets os.environ['CMDSTAN']
-        self.assertFalse(cmdstan_version_at(99, 99))
+        self.assertTrue(cmdstan_version_before(99, 99))
+        self.assertFalse(cmdstan_version_before(1, 1))
+
+    def test_cmdstan_version(self):
+        with tempfile.TemporaryDirectory(
+            prefix="cmdstan_tests", dir=_TMPDIR
+        ) as tmpdir:
+            tdir = os.path.join(tmpdir, 'tmpdir_xxx')
+            os.makedirs(tdir)
+            fake_path = os.path.join(tdir, 'cmdstan-2.22.0')
+            os.makedirs(os.path.join(fake_path))
+            fake_bin = os.path.join(fake_path, 'bin')
+            os.makedirs(fake_bin)
+            Path(os.path.join(fake_bin, 'stanc' + EXTENSION)).touch()
+            os.environ['CMDSTAN'] = fake_path
+            self.assertTrue(fake_path == cmdstan_path())
+            expect = (
+                'CmdStan installation {} missing makefile, '
+                'cannot get version.'.format(fake_path)
+            )
+            with LogCapture() as log:
+                logging.getLogger()
+                cmdstan_version()
+            log.check_present(('cmdstanpy', 'INFO', expect))
+            fake_makefile = os.path.join(fake_path, 'makefile')
+            with open(fake_makefile, 'w') as fd:
+                fd.write('...  CMDSTAN_VERSION := dont_need_no_mmp\n\n')
+            expect = (
+                'Cannot parse version, expected "<major>.<minor>.<patch>", '
+                'found: "dont_need_no_mmp".'
+            )
+            with LogCapture() as log:
+                logging.getLogger()
+                cmdstan_version()
+            log.check_present(('cmdstanpy', 'INFO', expect))
+        # cleanup
+        del os.environ['CMDSTAN']
+        cmdstan_path()
 
 
 class DataFilesTest(unittest.TestCase):