Skip to content
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.

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

Add test of optional ACA limits #385

Merged
merged 2 commits into from
Aug 15, 2023
Merged
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
57 changes: 51 additions & 6 deletions proseco/tests/test_catalog.py
Original file line number Diff line number Diff line change
@@ -1,26 +1,25 @@
# Licensed under a 3-clause BSD style license - see LICENSE.rst
import copy
import os

import matplotlib
from Quaternion import Quat

matplotlib.use("agg") # noqa

import pickle
from pathlib import Path

import agasc
import matplotlib
import mica.starcheck
import numpy as np
import pytest
from Quaternion import Quat

from .. import characteristics as ACA
from ..catalog import ACATable, get_aca_catalog
from ..core import ACACatalogTable, StarsTable, includes_for_obsid
from ..fid import FidTable, get_fid_catalog
from .test_common import DARK40, OBS_INFO, STD_INFO, mod_std_info

# Ensure all plotting is to a non-interactive backend
matplotlib.pyplot.switch_backend("agg")

HAS_SC_ARCHIVE = Path(mica.starcheck.starcheck.FILES["data_root"]).exists()
TEST_COLS = "slot idx id type sz yang zang dim res halfw".split()

Expand Down Expand Up @@ -398,6 +397,52 @@ def test_big_sim_offset():
assert aca.target_name == "Target Name"


def test_optional_penalty_limit():
"""Check that the optional penalty limit is supported correctly."""
try:
_test_optional_penalty_limit()
finally:
# Reset ACA limits characteristics that get set above
ACA._set_aca_limits()
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, so real testing question -- this makes sense but would there be other advantages to using a fixture to do this instead? Or have you not used a fixture because you prefer to explicitly test that ACA._set_aca_limits gets back to the 3.48 values?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't exactly sure how to do this with a fixture and decided this was good enough.


# Back to values from conftest.py
assert ACA.chandra_models_version == "3.48"
assert ACA.aca_t_ccd_penalty_limit == -5.5


def _test_optional_penalty_limit():
import json

from ska_helpers import chandra_models, paths
from ska_helpers.utils import temp_env_var

version = "3.49"
repo_path = paths.chandra_models_repo_path()

with chandra_models.get_local_repo(repo_path, version) as (repo, repo_path_local):
path_aca_spec = (
Path(repo_path_local) / "chandra_models" / "xija" / "aca" / "aca_spec.json"
)
text_aca_spec = path_aca_spec.read_text()
aca_spec = json.loads(text_aca_spec)
del aca_spec["limits"]["aacccdpt"]["planning.penalty.high"]
repo.git.checkout("HEAD", b="test_branch")
path_aca_spec.write_text(json.dumps(aca_spec, indent=4))
repo.git.add(path_aca_spec)
repo.git.commit("-m", "test commit")
taldcroft marked this conversation as resolved.
Show resolved Hide resolved

with temp_env_var("CHANDRA_MODELS_REPO_DIR", repo_path_local):
with temp_env_var("CHANDRA_MODELS_DEFAULT_VERSION", "test_branch"):
ACA._set_aca_limits()
assert ACA.aca_t_ccd_penalty_limit is None
assert ACA.chandra_models_version == "test_branch"

aca = get_aca_catalog(**STD_INFO, dark=DARK40)
assert aca.t_ccd_acq == aca.t_ccd_eff_acq
assert aca.t_ccd_guide == aca.t_ccd_eff_guide
assert aca.t_ccd_penalty_limit is None


@pytest.mark.parametrize("call_t_ccd", [True, False])
def test_calling_with_t_ccd_acq_guide(call_t_ccd):
"""Test that calling get_aca_catalog with t_ccd or t_ccd_acq/guide args sets all
Expand Down