Skip to content

xija_gui_fit enhancements#146

Merged
jzuhone merged 47 commits intomasterfrom
gui_fit_enh
Sep 16, 2025
Merged

xija_gui_fit enhancements#146
jzuhone merged 47 commits intomasterfrom
gui_fit_enh

Conversation

@jzuhone
Copy link
Copy Markdown
Collaborator

@jzuhone jzuhone commented Sep 3, 2025

Description

This PR adds a number of enhancements to xija_gui_fit, cleans up the code a little, and adds documentation. The extensive list of new features is:

  • Added a legend and autoscaling for the solarheat pitch plots
  • Added a plot for the ACIS power state coefficients
  • Added a native menu bar for the application, and moved several of the buttons on the main window to it
  • Moved the "fit status" to the status bar at the bottom of the main window
  • Writing the model + data time series output to an ASCII file now uses the ECSV format
  • Added functionality to make a plot showing the evolution of the fit statistic with number of iterations
  • Made the box containing the main window plots scrollable
  • Made changes to several dialogs which took input start and stop times, ensuring that these were valid and raising an informative error if they weren't
  • Added a title for the column with the sliders in the parameter table

I have added complete documentation for xija_gui_fit in this PR as well, which also is the best place to take a look at the new features. I have a rendered version of the docs up at https://cxc.cfa.harvard.edu/acis/xija/gui_fit.html.

Otherwise, I made some stylistic / cleanup / bugfix changes:

  • Removed subclassing from object
  • The XijaModel was being added as an attribute to a number of subclasses when it wasn't necessary, tried to pare this down
  • Made a function name more pythonic
  • Modernized calls to super().__init__()
  • Removed some dead code
  • Added PyCharm's .idea directory to .gitignore
  • Fixed a help string in the ArgumentParser
  • Updated code for more recent versions of Matplotlib (post 3.7.0)
  • Removed unused arguments and out-of-date comments
  • Made it so that the old ACIS FP limits did not appear on various plots

Interface impacts

Technically speaking, given that xija_gui_fit is a "graphical user interface", there are lots of interface changes, but they are all documented here, and since it only fits thermal models, nothing here affects flight operations immediately or directly.

Testing

Unit tests are not really feasible for xija_gui_fit, so we must rely on extensive functional testing.

Unit tests

  • No unit tests

Functional tests

Carried out a full fitting workflow on both macOS and Linux, exercising all options.

@jzuhone
Copy link
Copy Markdown
Collaborator Author

jzuhone commented Sep 3, 2025

@taldcroft it looks like there is at least one (new?) ruff check here that we might want to ignore

@jeanconn
Copy link
Copy Markdown
Contributor

jeanconn commented Sep 4, 2025

We've updated the ska3 template ruff-base to at least ignore PLC0415 (so ruff-base.toml could be updated from that reference file as a first step).

Copy link
Copy Markdown

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

@taldcroft
Copy link
Copy Markdown
Member

@jzuhone - I gave this a spin and everything worked with one minor exception:

I don't understand "Moved the "fit status" to the status bar at the bottom of the main window". I did a fit of the ACA model and I can't see anything like "fit status" in the window. Am I just missing something?

Otherwise, this is awesome work!

Copy link
Copy Markdown
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.

Great!

@taldcroft
Copy link
Copy Markdown
Member

(Though the ruff issue should be cleaned up).

@jzuhone
Copy link
Copy Markdown
Collaborator Author

jzuhone commented Sep 11, 2025

@taldcroft:

I don't understand "Moved the "fit status" to the status bar at the bottom of the main window". I did a fit of the ACA model and I can't see anything like "fit status" in the window. Am I just missing something?

I moved the "BUSY..." indicator to the status bar at the bottom of the window, which appears while fitting:

image

In the future this could give more information.

@jzuhone
Copy link
Copy Markdown
Collaborator Author

jzuhone commented Sep 12, 2025

@taldcroft most of the ruff changes were satisfied by running ruff format or ruff check --fix--others I had to edit the code by hand, but I checked that the operations in question still work and now the ruff checks pass.

@jzuhone jzuhone merged commit 06627cb into master Sep 16, 2025
2 checks passed
@javierggt javierggt mentioned this pull request Nov 18, 2025
@javierggt javierggt mentioned this pull request Jan 20, 2026
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.

4 participants