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

Improvements #3

Merged
merged 20 commits into from
Oct 23, 2024
Merged

Improvements #3

merged 20 commits into from
Oct 23, 2024

Conversation

javierggt
Copy link
Contributor

@javierggt javierggt commented Oct 14, 2024

Description

This PR makes a bunch of improvements that make aperoll usable in a more realistic situation.

Features included in this PR:

  • read from json file with yoshi parameters
  • stars can be manually included/excluded in the catalog
  • show the star catalog from proseco directly in the main window
  • improve the star plot:
    • show the catalog idx
    • show a tooltip with AGASC info for each star
    • show bad acq stars with the usual tomato color
  • the ability to read a proseco pickle
  • the ability to export:
    • a pickle in the same format as proseco.
    • a tarball with sparkles output.

Interface impacts

Testing

Unit tests

  • No unit tests
  • Mac
  • Linux
  • Windows

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

No functional testing.

@taldcroft
Copy link
Member

@javierggt - this is not working for me in current ska3-flight:

(ska3) ➜  aperoll git:(improvements) python -m aperoll.scripts.aperoll_main
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/Users/aldcroft/git/aperoll/aperoll/scripts/aperoll_main.py", line 6, in <module>
    from aperoll.widgets.main_window import MainWindow
  File "/Users/aldcroft/git/aperoll/aperoll/widgets/main_window.py", line 7, in <module>
    import PyQt5.QtWebEngineWidgets as QtWe
ModuleNotFoundError: No module named 'PyQt5.QtWebEngineWidgets'

@javierggt
Copy link
Contributor Author

javierggt commented Oct 15, 2024

You could do

conda -c conda-forge pyqtwebengine

it should not install any other package.

@taldcroft
Copy link
Member

taldcroft commented Oct 15, 2024

Looks great! Quick comments (and sorry for putting this here as a comment, you can transfer to different issues as you like).

  • I can't figure out how to zoom in or out.
  • The timeout on the tooltip with AGASC info is not my preference. I want to be able to look at the values for as long as I need.
  • The tooltip should default to showing all columns in the proseco_agasc since any of them can influence star selection. Or at least have some context key like holding Shift to show the full entry.
  • The roll should be constrained to be within the allow off-nominal roll for the given date and ra, dec. (Or maybe the roll turns red if it is outside the constraint).
  • No need to display more than 5 digits in RA, Dec, Roll.
  • I'd like an easy way to input either proseco args or yoshi args via copy/paste (e.g. from a Slack conversation).
  • If possible I'd prefer a UI that shows all of the proseco kwargs at once. This gives an immediate visual of the full story. As a table it doesn't take up that much area.
  • Following on that, there should be a convenient way to enter every supported proseco kwarg (https://sot.github.io/proseco/), within reason. For instance there is no way to provide a dark map or the table of stars, but all the rest are reasonable.

As always, you should take my many feature requests as a sign that I think this tool can be very useful to our group.

@javierggt
Copy link
Contributor Author

All good suggestions. I have some of those in my todo list.

To zoom I use the wheel (or whatever equivalent), in the track pad, I use two fingers (same as scrolling).

The issue with the tooltip time is frustrating. It is too fast, and I could not find a way to increase it. Supposedly there is an argument for that, but it did nothing.

The arguments more one adds, the more complex the dialog. Not saying I don't want to add them, I just need to figure out how to add them without stealing the real estate from the two main widgets and making the thing overly complex. I would need a comprehensive list of things we want, so I do it only once.

@javierggt
Copy link
Contributor Author

I also think the quaternion should be displayed, and one should be able to edit it.

@taldcroft
Copy link
Member

The comprehensive list of inputs is here: https://sot.github.io/proseco/#arguments. Looking that over, many of the optional arguments are rarely used and it is true that we may not want to waste real estate on them by default. OTOH if you read in a sparkles pickle and it has include_ids_fid set, then you really want to know that. Hmm...

…ickle, export to sparkles HTML, remove unused options, set dither to float, and moved option processing into separate functions (get_parameters_from_yoshi, get_parameters_from_pickle)
@javierggt javierggt merged commit 987e7cb into main Oct 23, 2024
2 checks passed
@javierggt javierggt deleted the improvements branch October 23, 2024 00:11
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.

2 participants