Conversation
- changed it so StarView.catalog is never None, and added Catalog.reset instead. - Added context menu to hide/show FOV, catalog and centroids. - Added auto-proseco feature
There was a problem hiding this comment.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no suggestions.
Comments skipped due to low confidence (5)
aperoll/widgets/star_plot.py:180
- [nitpick] The method name get_moving is ambiguous. It should be renamed to is_moving.
def get_moving(self):
aperoll/widgets/star_plot.py:255
- [nitpick] The variable name incl_action is ambiguous. It should be renamed to include_acq_action.
incl_action = QtW.QAction("include acq", menu, checkable=True)
aperoll/widgets/star_plot.py:259
- [nitpick] The variable name excl_action is ambiguous. It should be renamed to exclude_acq_action.
excl_action = QtW.QAction("exclude acq", menu, checkable=True)
aperoll/widgets/star_plot.py:309
- The hardcoded string 'include slot' should be replaced with a constant or enum for better maintainability.
if centroid is not None and result.text().startswith("include slot"):
aperoll/widgets/star_plot.py:320
- The hardcoded string 'Show FOV' should be replaced with a constant or enum for better maintainability.
elif result.text() == "Show FOV":
aperoll/star_field_items.py
Outdated
| | (row > 511) | ||
| | (col < -511) | ||
| | (col > 511) | ||
| | (self._centroids["IMGFID"]) |
There was a problem hiding this comment.
Why are the fid light centroids excluded here? They are a bit less interesting, but if we're just trying to display where on the CCD stuff is happening it might make sense to just include them.
There was a problem hiding this comment.
yes, I will add them back. I will use the same symbol but in red.
There was a problem hiding this comment.
@jeanconn I just pushed this change in this branch (it was in a branch in the other PR). I am going to merge this PR. I have used this extensively and found no issues, and it is getting harder to keep track of things.
There was a problem hiding this comment.
If you want me to review again let me know.
Description
The purpose of this PR is to:
This PR makes the following changes:
star_field_positionthat is used by all items to recalculate their position when the scene attitude changes.StarFieldbehavior into a dataclass of typeStarFieldState. The idea is that this is the one place to find all run-time switches that the user can change.StarViewsoStarView.catalogis neverNone, and addsCatalog.resetinstead.Notes about testing/reviewing this PR
I would like to restrict this PR to the items mentioned above. I tried to keep the changes to the user interface as minimal as possible while still allowing me to toggle the item's visibility. This PR is not intended to make major changes in the UI. The main change is the addition of the context menu, which is not conspicuous. It should be functional though:
One thing to note is the
StarPLot.set_onboard_attitudefunction.Requirements
This requires the latest agasc master (after sot/agasc/pull/193).
For real-time/telemetry use, use sot/aca_view/pull/197
Interface impacts
Testing
Unit tests
Independent check of unit tests by [REVIEWER NAME]
Functional tests
The usual aperoll command to inspect catalogs
Usage within aca_view to inspect telemetry. in this case, I tried: showing/hiding different items, dragging the display, opening the star view before there is any telemetry. Note that the "auto-proseco" feature does not work in this case.