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

Use chandra_limits to obtain limits, fix failing tests due to other ska updates #12

Merged
merged 4 commits into from
May 15, 2024

Conversation

jzuhone
Copy link
Member

@jzuhone jzuhone commented May 9, 2024

Description

The chandra_limits package is now operational. There is a function get_acis_limits that now no longer exists in acis_thermal_check, which was being used in acispy to obtain limits for the SimulateECSRun class. The fact that this function no longer exists was causing ImportErrors.

This PR removes the faulty import and fixes SimulateECSRun to use the new chandra_limits way of obtaining the limits.

Two other tests were failing because of recent updates to kadi, which resulted in very small changes to temperature predictions. These predictions have been updated since the reason is known. Another test was failing due to a change in xija, and it has been updated to pass.

Interface impacts

None

Testing

Unit tests

  • No unit tests
  • Mac
  • Linux
  • Windows

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

Ran a few instances of SimulateECSRun to make sure it ran correctly.

@jzuhone jzuhone requested review from Gregg140 and jazan12 May 9, 2024 18:50
Copy link

@jazan12 jazan12 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Copy link

@Gregg140 Gregg140 left a comment

Choose a reason for hiding this comment

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

Looks very good. Thanks for the fix.

@jzuhone jzuhone merged commit 5e20420 into master May 15, 2024
@jeanconn
Copy link

@jzuhone Does this update also need to be in the ska3 release to support the new ACIS FP limits or is this unrelated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants