From 9c96e0a3ea6cac00e309000b7995740eb5dc1f05 Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Wed, 30 Sep 2020 15:41:25 -0400
Subject: [PATCH 1/9] Add get_model_spec module for API access to model spec
 files

---
 docs/api.rst                      |   6 ++
 xija/get_model_spec.py            | 164 ++++++++++++++++++++++++++++++
 xija/tests/test_get_model_spec.py |  54 ++++++++++
 3 files changed, 224 insertions(+)
 create mode 100644 xija/get_model_spec.py
 create mode 100644 xija/tests/test_get_model_spec.py

diff --git a/docs/api.rst b/docs/api.rst
index 26ca3bd6..d8b8bf8c 100644
--- a/docs/api.rst
+++ b/docs/api.rst
@@ -27,3 +27,9 @@ Mask classes
 
 .. automodule:: xija.component.mask
    :members:
+
+Get model spec
+--------------
+.. automodule:: xija.get_model_spec
+   :members:
+
diff --git a/xija/get_model_spec.py b/xija/get_model_spec.py
new file mode 100644
index 00000000..666c536b
--- /dev/null
+++ b/xija/get_model_spec.py
@@ -0,0 +1,164 @@
+# Licensed under a 3-clause BSD style license - see LICENSE.rst
+"""
+Get Chandra model specifications
+"""
+import os
+import re
+from pathlib import Path
+from typing import List, Optional
+
+from git import Repo
+import requests
+from Ska.File import get_globfiles
+
+__all__ = ['get_xija_model_file', 'get_xija_model_names', 'get_repo_version',
+           'check_github_version']
+
+REPO_PATH = Path(os.environ['SKA'], 'data', 'chandra_models')
+MODELS_PATH = REPO_PATH / 'chandra_models' / 'xija'
+CHANDRA_MODELS_URL = 'https://api.github.com/repos/sot/chandra_models/releases'
+
+
+def get_xija_model_file(model_name, models_path=MODELS_PATH) -> str:
+    """
+    Get file name of Xija model specification for the specified ``model_name``.
+
+    Supported model names include (but are not limited to): ``'aca'``,
+    ``'acisfp'``, ``'dea'``, ``'dpa'``, ``'psmc'``, ``'minusyz'``, and
+    ``'pftank2t'``.
+
+    Use ``get_xija_model_names()`` for the full list.
+
+    Examples
+    --------
+    >>> import xija
+    >>> from xija.get_model_spec import get_xija_model_file
+    >>> model_spec = get_xija_model_file('acisfp')
+    >>> model = xija.XijaModel('acisfp', model_spec=model_spec, start='2012:001', stop='2012:010')
+    >>> model.make()
+    >>> model.calc()
+
+    Parameters
+    ----------
+    model_name : str
+        Name of model
+    models_path : str, Path
+        Path to directory containing xija model spec files (default is
+        $SKA/data/chandra_models)
+
+    Returns
+    -------
+    str
+        File name of the corresponding Xija model specification
+    """
+    models_path = Path(models_path)
+
+    if not models_path.exists():
+        raise FileNotFoundError(f'xija models directory {models_path} does not exist')
+
+    file_glob = str(models_path / '*' / f'{model_name.lower()}_spec.json')
+    try:
+        # get_globfiles() default requires exactly one file match and returns a list
+        file_name = get_globfiles(file_glob)[0]
+    except ValueError:
+        names = get_xija_model_names()
+        raise ValueError(f'no models matched {model_name}. Available models are: '
+                         f'{", ".join(names)}')
+
+    return file_name
+
+
+def get_xija_model_names(models_path=MODELS_PATH) -> List[str]:
+    """Return list of available xija model names.
+
+    Parameters
+    ----------
+    models_path : str, Path
+        Path to directory containing xija model spec files (default is
+        $SKA/data/chandra_models)
+
+    Examples
+    --------
+    >>> from xija.get_model_spec import get_xija_model_names
+    >>> names = get_xija_model_names()
+    ['aca',
+     'acisfp',
+     'dea',
+     'dpa',
+     '4rt700t',
+     'minusyz',
+     'pm1thv2t',
+     'pm2thv1t',
+     'pm2thv2t',
+     'pftank2t',
+     'pline03t_model',
+     'pline04t_model',
+     'psmc',
+     'tcylaft6']
+
+    Returns
+    -------
+    list
+        List of available xija model names
+    """
+    models_path = Path(models_path)
+
+    fns = get_globfiles(str(models_path / '*' / '*_spec.json'), minfiles=0, maxfiles=None)
+    names = [re.sub(r'_spec\.json', '', Path(fn).name) for fn in sorted(fns)]
+
+    return names
+
+
+def get_repo_version() -> str:
+    """Return version (most recent tag) of models repository.
+
+    Returns
+    -------
+    str
+        Version (most recent tag) of models repository
+    """
+    repo = Repo(REPO_PATH)
+
+    if repo.is_dirty():
+        raise ValueError('repo is dirty')
+
+    tags = sorted(repo.tags, key=lambda tag: tag.commit.committed_datetime)
+    tag_repo = tags[-1]
+    if tag_repo.commit != repo.head.commit:
+        raise ValueError(f'repo tip is not at tag {tag_repo}')
+
+    return tag_repo.name
+
+
+def check_github_version(tag_name, url=CHANDRA_MODELS_URL, timeout=5) -> Optional[bool]:
+    """Check that latest chandra_models GitHub repo release matches ``tag_name``.
+
+    This queries GitHub for the latest release of chandra_models.
+
+    Parameters
+    ----------
+    tag_name : str
+        Tag name e.g. '3.32'
+    url : str
+        URL for chandra_models releases on GitHub API
+    timeout : int, float
+        Request timeout (sec, default=5)
+
+    Returns
+    -------
+    bool, None
+        True if chandra_models release on GitHub matches tag_name.
+        None if the request timed out, indicating indeterminate answer.
+    """
+    try:
+        req = requests.get(url, timeout=timeout)
+    except requests.ConnectTimeout:
+        return None
+
+    if req.status_code != requests.codes.ok:
+        req.raise_for_status()
+
+    tags_gh = sorted(req.json(), key=lambda tag: tag['published_at'])
+    tag_gh_name = tags_gh[-1]['tag_name']
+
+    return tag_gh_name == tag_name
diff --git a/xija/tests/test_get_model_spec.py b/xija/tests/test_get_model_spec.py
new file mode 100644
index 00000000..ec8c2af8
--- /dev/null
+++ b/xija/tests/test_get_model_spec.py
@@ -0,0 +1,54 @@
+# Licensed under a 3-clause BSD style license - see LICENSE.rst
+
+import os
+import json
+from pathlib import Path
+import re
+import pytest
+import requests
+
+from ..get_model_spec import (get_xija_model_file, get_xija_model_names,
+                              get_repo_version, check_github_version)
+
+
+def test_get_model_file_aca():
+    fn = get_xija_model_file('aca')
+    assert fn.startswith(os.environ['SKA'])
+    assert Path(fn).name == 'aca_spec.json'
+    spec = json.load(open(fn))
+    assert spec['name'] == 'aacccdpt'
+
+
+def test_get_model_file_fail():
+    with pytest.raises(ValueError, match='no models matched xxxyyyzzz'):
+        get_xija_model_file('xxxyyyzzz')
+
+    with pytest.raises(FileNotFoundError, match='xija models directory'):
+        get_xija_model_file('aca', models_path='__NOT_A_DIRECTORY__')
+
+
+def test_get_xija_model_names():
+    names = get_xija_model_names()
+    assert all(name in names for name in ('aca', 'acisfp', 'dea', 'dpa', 'pftank2t'))
+
+
+def test_get_repo_version():
+    version = get_repo_version()
+    assert isinstance(version, str)
+    assert re.match(r'^[0-9.]+$', version)
+
+
+def test_check_github_version():
+    version = get_repo_version()
+    status = check_github_version(version)
+    assert status is True
+
+    status = check_github_version('asdf')
+    assert status is False
+
+    # Force timeout
+    status = check_github_version(version, timeout=0.00001)
+    assert status is None
+
+    with pytest.raises(requests.ConnectionError):
+        check_github_version(version, 'https://______bad_url______')

From f1fa86f8c2b9f1cc83dd3bbb10b894eeda433feb Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Thu, 1 Oct 2020 20:30:43 -0400
Subject: [PATCH 2/9] Pass in repo_path not model_path

---
 xija/get_model_spec.py            | 37 ++++++++++++++++++-------------
 xija/tests/test_get_model_spec.py |  2 +-
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/xija/get_model_spec.py b/xija/get_model_spec.py
index 666c536b..9eeb99c3 100644
--- a/xija/get_model_spec.py
+++ b/xija/get_model_spec.py
@@ -5,7 +5,7 @@
 import os
 import re
 from pathlib import Path
-from typing import List, Optional
+from typing import List, Optional, Union
 
 from git import Repo
 import requests
@@ -19,7 +19,11 @@
 CHANDRA_MODELS_URL = 'https://api.github.com/repos/sot/chandra_models/releases'
 
 
-def get_xija_model_file(model_name, models_path=MODELS_PATH) -> str:
+def _models_path(repo_path=REPO_PATH) -> Path:
+    return Path(repo_path) / 'chandra_models' / 'xija'
+
+
+def get_xija_model_file(model_name, repo_path=REPO_PATH) -> str:
     """
     Get file name of Xija model specification for the specified ``model_name``.
 
@@ -42,8 +46,8 @@ def get_xija_model_file(model_name, models_path=MODELS_PATH) -> str:
     ----------
     model_name : str
         Name of model
-    models_path : str, Path
-        Path to directory containing xija model spec files (default is
+    repo_path : str, Path
+        Path to directory containing chandra_models repository (default is
         $SKA/data/chandra_models)
 
     Returns
@@ -51,7 +55,7 @@ def get_xija_model_file(model_name, models_path=MODELS_PATH) -> str:
     str
         File name of the corresponding Xija model specification
     """
-    models_path = Path(models_path)
+    models_path = _models_path(repo_path)
 
     if not models_path.exists():
         raise FileNotFoundError(f'xija models directory {models_path} does not exist')
@@ -68,15 +72,9 @@ def get_xija_model_file(model_name, models_path=MODELS_PATH) -> str:
     return file_name
 
 
-def get_xija_model_names(models_path=MODELS_PATH) -> List[str]:
+def get_xija_model_names(repo_path=REPO_PATH) -> List[str]:
     """Return list of available xija model names.
 
-    Parameters
-    ----------
-    models_path : str, Path
-        Path to directory containing xija model spec files (default is
-        $SKA/data/chandra_models)
-
     Examples
     --------
     >>> from xija.get_model_spec import get_xija_model_names
@@ -96,12 +94,18 @@ def get_xija_model_names(models_path=MODELS_PATH) -> List[str]:
      'psmc',
      'tcylaft6']
 
+    Parameters
+    ----------
+    repo_path : str, Path
+        Path to directory containing chandra_models repository (default is
+        $SKA/data/chandra_models)
+
     Returns
     -------
     list
         List of available xija model names
     """
-    models_path = Path(models_path)
+    models_path = _models_path(repo_path)
 
     fns = get_globfiles(str(models_path / '*' / '*_spec.json'), minfiles=0, maxfiles=None)
     names = [re.sub(r'_spec\.json', '', Path(fn).name) for fn in sorted(fns)]
@@ -109,7 +113,7 @@ def get_xija_model_names(models_path=MODELS_PATH) -> List[str]:
     return names
 
 
-def get_repo_version() -> str:
+def get_repo_version(repo_path: Path = REPO_PATH) -> str:
     """Return version (most recent tag) of models repository.
 
     Returns
@@ -117,7 +121,7 @@ def get_repo_version() -> str:
     str
         Version (most recent tag) of models repository
     """
-    repo = Repo(REPO_PATH)
+    repo = Repo(repo_path)
 
     if repo.is_dirty():
         raise ValueError('repo is dirty')
@@ -130,7 +134,8 @@ def get_repo_version() -> str:
     return tag_repo.name
 
 
-def check_github_version(tag_name, url=CHANDRA_MODELS_URL, timeout=5) -> Optional[bool]:
+def check_github_version(tag_name: str, url: str = CHANDRA_MODELS_URL,
+                         timeout: Union[int, float] = 5) -> Optional[bool]:
     """Check that latest chandra_models GitHub repo release matches ``tag_name``.
 
     This queries GitHub for the latest release of chandra_models.
diff --git a/xija/tests/test_get_model_spec.py b/xija/tests/test_get_model_spec.py
index ec8c2af8..e9a26aa3 100644
--- a/xija/tests/test_get_model_spec.py
+++ b/xija/tests/test_get_model_spec.py
@@ -24,7 +24,7 @@ def test_get_model_file_fail():
         get_xija_model_file('xxxyyyzzz')
 
     with pytest.raises(FileNotFoundError, match='xija models directory'):
-        get_xija_model_file('aca', models_path='__NOT_A_DIRECTORY__')
+        get_xija_model_file('aca', repo_path='__NOT_A_DIRECTORY__')
 
 
 def test_get_xija_model_names():

From 95bcdd851ef3959782ecec49cc23978556089dff Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Thu, 1 Oct 2020 21:13:03 -0400
Subject: [PATCH 3/9] Check repo integrity always and optionally that GitHub
 matches

---
 xija/get_model_spec.py            | 37 ++++++++++++++++++++++---------
 xija/tests/test_get_model_spec.py | 20 ++++++++++++-----
 2 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/xija/get_model_spec.py b/xija/get_model_spec.py
index 9eeb99c3..df65adfe 100644
--- a/xija/get_model_spec.py
+++ b/xija/get_model_spec.py
@@ -4,6 +4,7 @@
 """
 import os
 import re
+import warnings
 from pathlib import Path
 from typing import List, Optional, Union
 
@@ -12,7 +13,7 @@
 from Ska.File import get_globfiles
 
 __all__ = ['get_xija_model_file', 'get_xija_model_names', 'get_repo_version',
-           'check_github_version']
+           'get_github_version']
 
 REPO_PATH = Path(os.environ['SKA'], 'data', 'chandra_models')
 MODELS_PATH = REPO_PATH / 'chandra_models' / 'xija'
@@ -23,7 +24,8 @@ def _models_path(repo_path=REPO_PATH) -> Path:
     return Path(repo_path) / 'chandra_models' / 'xija'
 
 
-def get_xija_model_file(model_name, repo_path=REPO_PATH) -> str:
+def get_xija_model_file(model_name, repo_path=REPO_PATH, check_version=False,
+                        timeout=5) -> str:
     """
     Get file name of Xija model specification for the specified ``model_name``.
 
@@ -49,6 +51,11 @@ def get_xija_model_file(model_name, repo_path=REPO_PATH) -> str:
     repo_path : str, Path
         Path to directory containing chandra_models repository (default is
         $SKA/data/chandra_models)
+    check_version : bool
+        Check that the chandra_models repo at the same version as GitHub
+    timeout : int, float
+        Timeout (sec) for querying GitHub for the expected chandra_models version.
+        Default = 5 sec.
 
     Returns
     -------
@@ -69,6 +76,18 @@ def get_xija_model_file(model_name, repo_path=REPO_PATH) -> str:
         raise ValueError(f'no models matched {model_name}. Available models are: '
                          f'{", ".join(names)}')
 
+    # Get version and also check that local repo is clean and tag is at tip of HEAD
+    repo_version = get_repo_version(repo_path)
+
+    if check_version:
+        gh_version = get_github_version(timeout=timeout)
+        if gh_version is None:
+            warnings.warn('Could not verify GitHub chandra_models release tag '
+                          f'due to timeout ({timeout} sec)')
+        elif repo_version != gh_version:
+            raise ValueError(f'version mismatch: local repo {repo_version} vs '
+                             f'github {gh_version}')
+
     return file_name
 
 
@@ -134,16 +153,14 @@ def get_repo_version(repo_path: Path = REPO_PATH) -> str:
     return tag_repo.name
 
 
-def check_github_version(tag_name: str, url: str = CHANDRA_MODELS_URL,
-                         timeout: Union[int, float] = 5) -> Optional[bool]:
-    """Check that latest chandra_models GitHub repo release matches ``tag_name``.
+def get_github_version(url: str = CHANDRA_MODELS_URL,
+                       timeout: Union[int, float] = 5) -> Optional[bool]:
+    """Get latest chandra_models GitHub repo release tag (version).
 
     This queries GitHub for the latest release of chandra_models.
 
     Parameters
     ----------
-    tag_name : str
-        Tag name e.g. '3.32'
     url : str
         URL for chandra_models releases on GitHub API
     timeout : int, float
@@ -151,8 +168,8 @@ def check_github_version(tag_name: str, url: str = CHANDRA_MODELS_URL,
 
     Returns
     -------
-    bool, None
-        True if chandra_models release on GitHub matches tag_name.
+    str, None
+        Tag name (str) of latest chandra_models release on GitHub.
         None if the request timed out, indicating indeterminate answer.
     """
     try:
@@ -166,4 +183,4 @@ def check_github_version(tag_name: str, url: str = CHANDRA_MODELS_URL,
     tags_gh = sorted(req.json(), key=lambda tag: tag['published_at'])
     tag_gh_name = tags_gh[-1]['tag_name']
 
-    return tag_gh_name == tag_name
+    return tag_gh_name
diff --git a/xija/tests/test_get_model_spec.py b/xija/tests/test_get_model_spec.py
index e9a26aa3..fef527a9 100644
--- a/xija/tests/test_get_model_spec.py
+++ b/xija/tests/test_get_model_spec.py
@@ -8,11 +8,18 @@
 import requests
 
 from ..get_model_spec import (get_xija_model_file, get_xija_model_names,
-                              get_repo_version, check_github_version)
+                              get_repo_version, get_github_version)
+
+try:
+    req = requests.get('https://raw.githubusercontent.com/sot/chandra_models/master/README',
+                       timeout=5)
+    HAS_GITHUB = req.status_code == 200
+except Exception:
+    HAS_GITHUB = False
 
 
 def test_get_model_file_aca():
-    fn = get_xija_model_file('aca')
+    fn = get_xija_model_file('aca', check_version=HAS_GITHUB)
     assert fn.startswith(os.environ['SKA'])
     assert Path(fn).name == 'aca_spec.json'
     spec = json.load(open(fn))
@@ -38,17 +45,18 @@ def test_get_repo_version():
     assert re.match(r'^[0-9.]+$', version)
 
 
+@pytest.mark.skipif('not HAS_GITHUB')
 def test_check_github_version():
     version = get_repo_version()
-    status = check_github_version(version)
+    status = get_github_version() == version
     assert status is True
 
-    status = check_github_version('asdf')
+    status = get_github_version() == 'asdf'
     assert status is False
 
     # Force timeout
-    status = check_github_version(version, timeout=0.00001)
+    status = get_github_version(timeout=0.00001)
     assert status is None
 
     with pytest.raises(requests.ConnectionError):
-        check_github_version(version, 'https://______bad_url______')
+        get_github_version(url='https://______bad_url______')

From 62b4aaeac3f9df2578be9f71022b529379b63a04 Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Fri, 2 Oct 2020 06:12:48 -0400
Subject: [PATCH 4/9] Get model spec as dict instead of filename

---
 xija/get_model_spec.py            | 38 ++++++++++++++++---------------
 xija/tests/test_get_model_spec.py | 14 +++++-------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/xija/get_model_spec.py b/xija/get_model_spec.py
index df65adfe..01ad2590 100644
--- a/xija/get_model_spec.py
+++ b/xija/get_model_spec.py
@@ -2,6 +2,7 @@
 """
 Get Chandra model specifications
 """
+import json
 import os
 import re
 import warnings
@@ -12,7 +13,7 @@
 import requests
 from Ska.File import get_globfiles
 
-__all__ = ['get_xija_model_file', 'get_xija_model_names', 'get_repo_version',
+__all__ = ['get_xija_model_spec', 'get_xija_model_names', 'get_repo_version',
            'get_github_version']
 
 REPO_PATH = Path(os.environ['SKA'], 'data', 'chandra_models')
@@ -24,10 +25,10 @@ def _models_path(repo_path=REPO_PATH) -> Path:
     return Path(repo_path) / 'chandra_models' / 'xija'
 
 
-def get_xija_model_file(model_name, repo_path=REPO_PATH, check_version=False,
-                        timeout=5) -> str:
+def get_xija_model_spec(model_name, repo_path=REPO_PATH, check_version=False,
+                        timeout=5) -> dict:
     """
-    Get file name of Xija model specification for the specified ``model_name``.
+    Get Xija model specification for the specified ``model_name``.
 
     Supported model names include (but are not limited to): ``'aca'``,
     ``'acisfp'``, ``'dea'``, ``'dpa'``, ``'psmc'``, ``'minusyz'``, and
@@ -38,8 +39,8 @@ def get_xija_model_file(model_name, repo_path=REPO_PATH, check_version=False,
     Examples
     --------
     >>> import xija
-    >>> from xija.get_model_spec import get_xija_model_file
-    >>> model_spec = get_xija_model_file('acisfp')
+    >>> from xija.get_model_spec import get_xija_model_spec
+    >>> model_spec = get_xija_model_spec('acisfp')
     >>> model = xija.XijaModel('acisfp', model_spec=model_spec, start='2012:001', stop='2012:010')
     >>> model.make()
     >>> model.calc()
@@ -59,23 +60,14 @@ def get_xija_model_file(model_name, repo_path=REPO_PATH, check_version=False,
 
     Returns
     -------
-    str
-        File name of the corresponding Xija model specification
+    dict
+        Xija model specification dict
     """
     models_path = _models_path(repo_path)
 
     if not models_path.exists():
         raise FileNotFoundError(f'xija models directory {models_path} does not exist')
 
-    file_glob = str(models_path / '*' / f'{model_name.lower()}_spec.json')
-    try:
-        # get_globfiles() default requires exactly one file match and returns a list
-        file_name = get_globfiles(file_glob)[0]
-    except ValueError:
-        names = get_xija_model_names()
-        raise ValueError(f'no models matched {model_name}. Available models are: '
-                         f'{", ".join(names)}')
-
     # Get version and also check that local repo is clean and tag is at tip of HEAD
     repo_version = get_repo_version(repo_path)
 
@@ -88,7 +80,17 @@ def get_xija_model_file(model_name, repo_path=REPO_PATH, check_version=False,
             raise ValueError(f'version mismatch: local repo {repo_version} vs '
                              f'github {gh_version}')
 
-    return file_name
+    file_glob = str(models_path / '*' / f'{model_name.lower()}_spec.json')
+    try:
+        # get_globfiles() default requires exactly one file match and returns a list
+        file_name = get_globfiles(file_glob)[0]
+    except ValueError:
+        names = get_xija_model_names()
+        raise ValueError(f'no models matched {model_name}. Available models are: '
+                         f'{", ".join(names)}')
+
+    model_spec = json.load(open(file_name, 'r'))
+    return model_spec
 
 
 def get_xija_model_names(repo_path=REPO_PATH) -> List[str]:
diff --git a/xija/tests/test_get_model_spec.py b/xija/tests/test_get_model_spec.py
index fef527a9..d8d03cef 100644
--- a/xija/tests/test_get_model_spec.py
+++ b/xija/tests/test_get_model_spec.py
@@ -7,7 +7,7 @@
 import pytest
 import requests
 
-from ..get_model_spec import (get_xija_model_file, get_xija_model_names,
+from ..get_model_spec import (get_xija_model_spec, get_xija_model_names,
                               get_repo_version, get_github_version)
 
 try:
@@ -18,20 +18,18 @@
     HAS_GITHUB = False
 
 
-def test_get_model_file_aca():
-    fn = get_xija_model_file('aca', check_version=HAS_GITHUB)
-    assert fn.startswith(os.environ['SKA'])
-    assert Path(fn).name == 'aca_spec.json'
-    spec = json.load(open(fn))
+def test_get_model_spec_aca():
+    spec = get_xija_model_spec('aca', check_version=HAS_GITHUB)
     assert spec['name'] == 'aacccdpt'
+    assert 'comps' in spec
 
 
 def test_get_model_file_fail():
     with pytest.raises(ValueError, match='no models matched xxxyyyzzz'):
-        get_xija_model_file('xxxyyyzzz')
+        get_xija_model_spec('xxxyyyzzz')
 
     with pytest.raises(FileNotFoundError, match='xija models directory'):
-        get_xija_model_file('aca', repo_path='__NOT_A_DIRECTORY__')
+        get_xija_model_spec('aca', repo_path='__NOT_A_DIRECTORY__')
 
 
 def test_get_xija_model_names():

From 1553cbf9bfa394a2403a261e07f1078ccee8ea83 Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Fri, 2 Oct 2020 06:41:38 -0400
Subject: [PATCH 5/9] Initial refactor to allow getting different version of
 spec

---
 xija/get_model_spec.py            | 44 ++++++++++++++++++++-----------
 xija/tests/test_get_model_spec.py |  2 +-
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/xija/get_model_spec.py b/xija/get_model_spec.py
index 01ad2590..4696ebcd 100644
--- a/xija/get_model_spec.py
+++ b/xija/get_model_spec.py
@@ -25,8 +25,8 @@ def _models_path(repo_path=REPO_PATH) -> Path:
     return Path(repo_path) / 'chandra_models' / 'xija'
 
 
-def get_xija_model_spec(model_name, repo_path=REPO_PATH, check_version=False,
-                        timeout=5) -> dict:
+def get_xija_model_spec(model_name, version=None, repo_path=REPO_PATH,
+                        check_version=False, timeout=5) -> dict:
     """
     Get Xija model specification for the specified ``model_name``.
 
@@ -49,11 +49,13 @@ def get_xija_model_spec(model_name, repo_path=REPO_PATH, check_version=False,
     ----------
     model_name : str
         Name of model
+    version : str
+        Version of chandra_models to use (default=latest from local repo)
     repo_path : str, Path
         Path to directory containing chandra_models repository (default is
         $SKA/data/chandra_models)
     check_version : bool
-        Check that the chandra_models repo at the same version as GitHub
+        Check that ``version`` matches the latest release on GitHub
     timeout : int, float
         Timeout (sec) for querying GitHub for the expected chandra_models version.
         Default = 5 sec.
@@ -63,23 +65,22 @@ def get_xija_model_spec(model_name, repo_path=REPO_PATH, check_version=False,
     dict
         Xija model specification dict
     """
+    return _get_xija_model_spec(model_name, repo_path, check_version, timeout)
+
+
+def _get_xija_model_spec(model_name, repo_path=REPO_PATH,
+                         check_version=False, timeout=5) -> dict:
+
+    repo_path = Path(repo_path)
+
+    if not repo_path.exists():
+        raise FileNotFoundError(f'chandra_models repository {repo_path} does not exist')
+
     models_path = _models_path(repo_path)
 
     if not models_path.exists():
         raise FileNotFoundError(f'xija models directory {models_path} does not exist')
 
-    # Get version and also check that local repo is clean and tag is at tip of HEAD
-    repo_version = get_repo_version(repo_path)
-
-    if check_version:
-        gh_version = get_github_version(timeout=timeout)
-        if gh_version is None:
-            warnings.warn('Could not verify GitHub chandra_models release tag '
-                          f'due to timeout ({timeout} sec)')
-        elif repo_version != gh_version:
-            raise ValueError(f'version mismatch: local repo {repo_version} vs '
-                             f'github {gh_version}')
-
     file_glob = str(models_path / '*' / f'{model_name.lower()}_spec.json')
     try:
         # get_globfiles() default requires exactly one file match and returns a list
@@ -90,6 +91,19 @@ def get_xija_model_spec(model_name, repo_path=REPO_PATH, check_version=False,
                          f'{", ".join(names)}')
 
     model_spec = json.load(open(file_name, 'r'))
+
+    # Get version and ensure that repo is clean and tip is at latest tag
+    version = get_repo_version(repo_path)
+
+    if check_version:
+        gh_version = get_github_version(timeout=timeout)
+        if gh_version is None:
+            warnings.warn('Could not verify GitHub chandra_models release tag '
+                          f'due to timeout ({timeout} sec)')
+        elif version != gh_version:
+            raise ValueError(f'version mismatch: local repo {repo_version} vs '
+                             f'github {gh_version}')
+
     return model_spec
 
 
diff --git a/xija/tests/test_get_model_spec.py b/xija/tests/test_get_model_spec.py
index d8d03cef..9d2544ba 100644
--- a/xija/tests/test_get_model_spec.py
+++ b/xija/tests/test_get_model_spec.py
@@ -28,7 +28,7 @@ def test_get_model_file_fail():
     with pytest.raises(ValueError, match='no models matched xxxyyyzzz'):
         get_xija_model_spec('xxxyyyzzz')
 
-    with pytest.raises(FileNotFoundError, match='xija models directory'):
+    with pytest.raises(FileNotFoundError, match='chandra_models repository'):
         get_xija_model_spec('aca', repo_path='__NOT_A_DIRECTORY__')
 
 

From fcda0a263ec077fb9c39fc374dd06a5dcd775299 Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Fri, 2 Oct 2020 07:41:59 -0400
Subject: [PATCH 6/9] Allow for arbitrary version

---
 xija/get_model_spec.py | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/xija/get_model_spec.py b/xija/get_model_spec.py
index 4696ebcd..7ac923c0 100644
--- a/xija/get_model_spec.py
+++ b/xija/get_model_spec.py
@@ -3,6 +3,7 @@
 Get Chandra model specifications
 """
 import json
+import tempfile
 import os
 import re
 import warnings
@@ -65,17 +66,26 @@ def get_xija_model_spec(model_name, version=None, repo_path=REPO_PATH,
     dict
         Xija model specification dict
     """
-    return _get_xija_model_spec(model_name, repo_path, check_version, timeout)
-
-
-def _get_xija_model_spec(model_name, repo_path=REPO_PATH,
-                         check_version=False, timeout=5) -> dict:
-
     repo_path = Path(repo_path)
 
     if not repo_path.exists():
         raise FileNotFoundError(f'chandra_models repository {repo_path} does not exist')
 
+    if version is None:
+        return _get_xija_model_spec(model_name, version, repo_path, check_version, timeout)
+
+    repo_ska = Repo(repo_path)
+    with tempfile.TemporaryDirectory() as repo_path:
+        repo = repo_ska.clone(repo_path)
+        repo.git.checkout(version)
+        spec = _get_xija_model_spec(model_name, version, repo_path, check_version, timeout)
+
+    return spec
+
+
+def _get_xija_model_spec(model_name, version=None, repo_path=REPO_PATH,
+                         check_version=False, timeout=5) -> dict:
+
     models_path = _models_path(repo_path)
 
     if not models_path.exists():
@@ -93,7 +103,8 @@ def _get_xija_model_spec(model_name, repo_path=REPO_PATH,
     model_spec = json.load(open(file_name, 'r'))
 
     # Get version and ensure that repo is clean and tip is at latest tag
-    version = get_repo_version(repo_path)
+    if version is None:
+        version = get_repo_version(repo_path)
 
     if check_version:
         gh_version = get_github_version(timeout=timeout)
@@ -101,7 +112,7 @@ def _get_xija_model_spec(model_name, repo_path=REPO_PATH,
             warnings.warn('Could not verify GitHub chandra_models release tag '
                           f'due to timeout ({timeout} sec)')
         elif version != gh_version:
-            raise ValueError(f'version mismatch: local repo {repo_version} vs '
+            raise ValueError(f'version mismatch: local repo {version} vs '
                              f'github {gh_version}')
 
     return model_spec

From 4cf58a3499055aafa7173764255027c4a5264d32 Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Fri, 2 Oct 2020 09:45:01 -0400
Subject: [PATCH 7/9] More general version and allow URL for repo_path

---
 xija/get_model_spec.py            | 29 +++++++++++------------------
 xija/tests/test_get_model_spec.py |  7 +++----
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/xija/get_model_spec.py b/xija/get_model_spec.py
index 7ac923c0..5571b4d5 100644
--- a/xija/get_model_spec.py
+++ b/xija/get_model_spec.py
@@ -10,7 +10,7 @@
 from pathlib import Path
 from typing import List, Optional, Union
 
-from git import Repo
+import git
 import requests
 from Ska.File import get_globfiles
 
@@ -51,10 +51,11 @@ def get_xija_model_spec(model_name, version=None, repo_path=REPO_PATH,
     model_name : str
         Name of model
     version : str
-        Version of chandra_models to use (default=latest from local repo)
+        Tag, branch or commit of chandra_models to use (default=latest tag from
+        repo)
     repo_path : str, Path
-        Path to directory containing chandra_models repository (default is
-        $SKA/data/chandra_models)
+        Path to directory or URL containing chandra_models repository (default
+        is $SKA/data/chandra_models)
     check_version : bool
         Check that ``version`` matches the latest release on GitHub
     timeout : int, float
@@ -66,19 +67,11 @@ def get_xija_model_spec(model_name, version=None, repo_path=REPO_PATH,
     dict
         Xija model specification dict
     """
-    repo_path = Path(repo_path)
-
-    if not repo_path.exists():
-        raise FileNotFoundError(f'chandra_models repository {repo_path} does not exist')
-
-    if version is None:
-        return _get_xija_model_spec(model_name, version, repo_path, check_version, timeout)
-
-    repo_ska = Repo(repo_path)
-    with tempfile.TemporaryDirectory() as repo_path:
-        repo = repo_ska.clone(repo_path)
-        repo.git.checkout(version)
-        spec = _get_xija_model_spec(model_name, version, repo_path, check_version, timeout)
+    with tempfile.TemporaryDirectory() as repo_path_local:
+        repo = git.Repo.clone_from(repo_path, repo_path_local)
+        if version is not None:
+            repo.git.checkout(version)
+        spec = _get_xija_model_spec(model_name, version, repo_path_local, check_version, timeout)
 
     return spec
 
@@ -167,7 +160,7 @@ def get_repo_version(repo_path: Path = REPO_PATH) -> str:
     str
         Version (most recent tag) of models repository
     """
-    repo = Repo(repo_path)
+    repo = git.Repo(repo_path)
 
     if repo.is_dirty():
         raise ValueError('repo is dirty')
diff --git a/xija/tests/test_get_model_spec.py b/xija/tests/test_get_model_spec.py
index 9d2544ba..4ac3665b 100644
--- a/xija/tests/test_get_model_spec.py
+++ b/xija/tests/test_get_model_spec.py
@@ -1,11 +1,10 @@
 # Licensed under a 3-clause BSD style license - see LICENSE.rst
 
-import os
-import json
-from pathlib import Path
 import re
+
 import pytest
 import requests
+import git
 
 from ..get_model_spec import (get_xija_model_spec, get_xija_model_names,
                               get_repo_version, get_github_version)
@@ -28,7 +27,7 @@ def test_get_model_file_fail():
     with pytest.raises(ValueError, match='no models matched xxxyyyzzz'):
         get_xija_model_spec('xxxyyyzzz')
 
-    with pytest.raises(FileNotFoundError, match='chandra_models repository'):
+    with pytest.raises(git.GitCommandError, match='does not exist'):
         get_xija_model_spec('aca', repo_path='__NOT_A_DIRECTORY__')
 
 

From 169626139ce80e12d8648e9d0e36c66a017bf804 Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Fri, 2 Oct 2020 11:55:58 -0400
Subject: [PATCH 8/9] More improvements and tests

---
 xija/get_model_spec.py            | 27 +++++++++++++++++++--------
 xija/tests/test_get_model_spec.py | 26 ++++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/xija/get_model_spec.py b/xija/get_model_spec.py
index 5571b4d5..b9981869 100644
--- a/xija/get_model_spec.py
+++ b/xija/get_model_spec.py
@@ -39,13 +39,24 @@ def get_xija_model_spec(model_name, version=None, repo_path=REPO_PATH,
 
     Examples
     --------
+    Get the latest version of the ``acisfp`` model spec from the local Ska data
+    directory ``$SKA/data/chandra_models``, checking that the version matches
+    the latest release tag on GitHub.
+
     >>> import xija
     >>> from xija.get_model_spec import get_xija_model_spec
-    >>> model_spec = get_xija_model_spec('acisfp')
-    >>> model = xija.XijaModel('acisfp', model_spec=model_spec, start='2012:001', stop='2012:010')
+    >>> model_spec, version = get_xija_model_spec('acisfp', check_version=True)
+    >>> model = xija.XijaModel('acisfp', model_spec=model_spec,
+    ...                        start='2012:001', stop='2012:010')
     >>> model.make()
     >>> model.calc()
 
+    Get the ``aca`` model spec from release version 3.30 of chandra_models from
+    GitHub.
+
+    >>> repo_path = 'https://github.com/sot/chandra_models.git'
+    >>> model_spec, version = get_xija_model_spec('aca', version='3.30', repo_path=repo_path)
+
     Parameters
     ----------
     model_name : str
@@ -64,16 +75,16 @@ def get_xija_model_spec(model_name, version=None, repo_path=REPO_PATH,
 
     Returns
     -------
-    dict
-        Xija model specification dict
+    dict, str
+        Xija model specification dict, chandra_models version
     """
     with tempfile.TemporaryDirectory() as repo_path_local:
         repo = git.Repo.clone_from(repo_path, repo_path_local)
         if version is not None:
             repo.git.checkout(version)
-        spec = _get_xija_model_spec(model_name, version, repo_path_local, check_version, timeout)
-
-    return spec
+        model_spec, version = _get_xija_model_spec(model_name, version, repo_path_local,
+                                                   check_version, timeout)
+    return model_spec, version
 
 
 def _get_xija_model_spec(model_name, version=None, repo_path=REPO_PATH,
@@ -108,7 +119,7 @@ def _get_xija_model_spec(model_name, version=None, repo_path=REPO_PATH,
             raise ValueError(f'version mismatch: local repo {version} vs '
                              f'github {gh_version}')
 
-    return model_spec
+    return model_spec, version
 
 
 def get_xija_model_names(repo_path=REPO_PATH) -> List[str]:
diff --git a/xija/tests/test_get_model_spec.py b/xija/tests/test_get_model_spec.py
index 4ac3665b..c50fb01a 100644
--- a/xija/tests/test_get_model_spec.py
+++ b/xija/tests/test_get_model_spec.py
@@ -10,6 +10,7 @@
                               get_repo_version, get_github_version)
 
 try:
+    # Fast request to see if GitHub is available
     req = requests.get('https://raw.githubusercontent.com/sot/chandra_models/master/README',
                        timeout=5)
     HAS_GITHUB = req.status_code == 200
@@ -17,10 +18,31 @@
     HAS_GITHUB = False
 
 
-def test_get_model_spec_aca():
-    spec = get_xija_model_spec('aca', check_version=HAS_GITHUB)
+def test_get_model_spec_aca_3_30():
+    # Version 3.30
+    spec, version = get_xija_model_spec('aca', version='3.30')
     assert spec['name'] == 'aacccdpt'
     assert 'comps' in spec
+    assert spec["datestop"] == "2018:305:11:52:30.816"
+
+
+def test_get_model_spec_aca_latest():
+    # Latest version
+    spec, version = get_xija_model_spec('aca', check_version=HAS_GITHUB)
+    assert spec['name'] == 'aacccdpt'
+    # version 3.30 value, changed in 3.31.1/3.32
+    assert spec["datestop"] != "2018:305:11:52:30.816"
+    assert 'comps' in spec
+
+
+@pytest.mark.skipif('not HAS_GITHUB')
+def test_get_model_spec_aca_from_github():
+    # Latest version
+    repo_path = 'https://github.com/sot/chandra_models.git'
+    spec, version = get_xija_model_spec('aca', repo_path=repo_path, version='3.30')
+    assert spec['name'] == 'aacccdpt'
+    assert spec["datestop"] == "2018:305:11:52:30.816"
+    assert 'comps' in spec
 
 
 def test_get_model_file_fail():

From 957de381f35230386aefb07db483e8899c187920 Mon Sep 17 00:00:00 2001
From: Tom Aldcroft <taldcroft@gmail.com>
Date: Fri, 2 Oct 2020 12:04:55 -0400
Subject: [PATCH 9/9] Doc tweaks

---
 xija/get_model_spec.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xija/get_model_spec.py b/xija/get_model_spec.py
index b9981869..cbff42f6 100644
--- a/xija/get_model_spec.py
+++ b/xija/get_model_spec.py
@@ -47,7 +47,7 @@ def get_xija_model_spec(model_name, version=None, repo_path=REPO_PATH,
     >>> from xija.get_model_spec import get_xija_model_spec
     >>> model_spec, version = get_xija_model_spec('acisfp', check_version=True)
     >>> model = xija.XijaModel('acisfp', model_spec=model_spec,
-    ...                        start='2012:001', stop='2012:010')
+    ...                        start='2020:001', stop='2020:010')
     >>> model.make()
     >>> model.calc()
 
@@ -55,7 +55,8 @@ def get_xija_model_spec(model_name, version=None, repo_path=REPO_PATH,
     GitHub.
 
     >>> repo_path = 'https://github.com/sot/chandra_models.git'
-    >>> model_spec, version = get_xija_model_spec('aca', version='3.30', repo_path=repo_path)
+    >>> model_spec, version = get_xija_model_spec('aca', version='3.30',
+    ...                                           repo_path=repo_path)
 
     Parameters
     ----------