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.

Sign up for GitHub

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

Jump to bottom

Add new ACIS FP limit colors #139

Merged
merged 1 commit into from
May 10, 2024
Merged

Add new ACIS FP limit colors #139

merged 1 commit into from
May 10, 2024

Conversation

jzuhone
Copy link
Collaborator

@jzuhone jzuhone commented May 10, 2024 •
edited by taldcroft
Loading

Description

In preparation for the new ACIS FP limit scheme, this PR adds new limit colors to the limit color lookup dictionary.

Interface impacts

Colors of the new ACIS FP limits in xija_gui_fit and acis_thermal_check will change.

Testing

Unit tests

  • No unit tests
  • Mac
  • Linux
  • Windows

Independent check of unit tests by @taldcroft

  • Mac

Functional tests

Checked that the correct colors appear in acis_thermal_check.

taldcroft
taldcroft approved these changes May 10, 2024
View reviewed changes
Copy link
Member

@taldcroft taldcroft 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.

matthewdahmer
matthewdahmer approved these changes May 10, 2024
View reviewed changes
Copy link
Contributor

@matthewdahmer matthewdahmer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me!

jeanconn
jeanconn approved these changes May 10, 2024
View reviewed changes
Copy link
Contributor

@jeanconn jeanconn 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. Though as with everything we update in xija or the tools, I do wonder if it would make sense to eventually get this dictionary into chandra_models?

@jzuhone jzuhone merged commit 3166678 into master May 10, 2024
@javierggt javierggt mentioned this pull request May 15, 2024
Closed
@jeanconn jeanconn mentioned this pull request May 20, 2024
Merged
@javierggt javierggt mentioned this pull request Jun 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewdahmer matthewdahmer matthewdahmer approved these changes

@jeanconn jeanconn jeanconn approved these changes

@taldcroft taldcroft taldcroft approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jzuhone @taldcroft @jeanconn @matthewdahmer