Update seeing profile/aperture settings notebook to use new infrastructure #358

mwcraig · 2024-05-28T14:56:39Z

This PR does a few things:

Changes some of the layout of the seeing profile/aperture settings notebook
Adds a camera selector(fixes Notebook 01 needs to be modified to take a Camera #246)
Adds an error console to that notebook to tell the user when they need to click on a star again.
Saves aperture settings and camera to a partial photometry settings file.
Does some internal variable renaming to make the names more meaningful.
Adds a launcher entry for this notebook.

Some things to try:

Hit the launcher to get a copy of this notebook.
Load an image, zoom way in and shift-click far from a star. An error message should be displayed above the image.
Next, shift-click on a star. You should get some plots, and some suggested aperture settings.
Save the aperture settings and verify they appear properly in the json file created by this notebook.

JuanCab · 2024-05-28T23:03:50Z

To perform the initial review of the functionality of this PR, I entered the following commands on the command line:

conda create -n test-358 python=3.11 astroscrappy
conda activate test-358
pip install git+https://github.com/mwcraig/stellarphot.git@camera-nb-01
jupyter lab

Addressing some of your suggested "things to try:"

Hit the launcher to get a copy of this notebook.

This worked fine although you didn't clarify WHICH notebook to copy (there were two listed). I assumed it was the "Seeing Profile" from the context clues.
Load an image, zoom way in and shift-click far from a star. An error message should be displayed above the image.
Yep, "No star found at this location" error was displayed, but not when I "shift-clicked," I had to click without holding down the shift! One potential issue, if you click at several locations far from stars, multiple error messages show up and they actually push the image down the page.
Next, shift-click on a star. You should get some plots, and some suggested aperture settings.
Again, I had to just click and not shift-click, but yes, profiles showed up.

Save the aperture settings and verify they appear properly in the json file created by this notebook.
I saved the settings and the following (apparently proper JSON content) was saved:

{
    "camera": {
        "name": "Testing",
        "data_unit": "adu",
        "gain": "10.0 electron / adu",
        "read_noise": "10.0 electron",
        "dark_current": "0.01 electron / s",
        "pixel_scale": "0.6 arcsec / pix",
        "max_data_value": "50000.0 adu"
    },
    "observatory": null,
    "passband_map": null,
    "photometry_apertures": {
        "radius": 8,
        "gap": 5,
        "annulus_width": 15,
        "fwhm": 5.650264631520387
    },
    "source_locations": null,
    "photometry_optional_settings": null,
    "logging_settings": null
}

mwcraig · 2024-05-29T14:52:37Z

The broken test should be fixed now, @JuanCab. Not sure I like the fix, which was to save a camera if it is passed into the seeing profile widget and is not already saved...

codecov · 2024-05-29T14:56:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.42%. Comparing base (312fc0c) to head (e13ddc2).
Report is 308 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #358      +/-   ##
==========================================
+ Coverage   75.25%   76.42%   +1.17%     
==========================================
  Files          27       27              
  Lines        3601     3657      +56     
==========================================
+ Hits         2710     2795      +85     
+ Misses        891      862      -29

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

JuanCab · 2024-05-29T18:34:00Z

I'll note again this issue (stated above): "if you click at several locations far from stars, multiple error messages show up and they actually push the image down the page."

JuanCab · 2024-05-29T18:38:45Z

stellarphot/gui_tools/seeing_profile_functions.py

@@ -256,6 +306,29 @@ def load_fits(self):
            )
            self.exposure = np.nan

+    def save(self):


Looks like this function/method is not tested according to codecov, I am seeing a bunch of warnings about that inline with the code review.

About to push a fix for this....

JuanCab · 2024-05-29T18:40:33Z

stellarphot/gui_tools/seeing_profile_functions.py

-        self.aperture_settings.savebuttonbar.unsaved_changes = True
+    def _set_save_box_title(self, _):
+        if self.aperture_settings.savebuttonbar.unsaved_changes:
+            self.ap_title.value = self._format_title(DEFAULT_SAVE_TITLE + " ❗️")


Just for maintainability, couldn't these two lines that change the self.ap_title.value use the AP_SETTING_NEEDS_SAVE and AP_SETTING_SAVED constants you defined? Might even make the code more readable. :)

Used, including in the incoming tests

stellarphot/gui_tools/seeing_profile_functions.py

stellarphot/notebooks/jp_app_launcher_stellarphot.yaml

stellarphot/settings/tests/test_custom_widgets.py

Tanner728

Here is my review of the code mostly focusing on the UI/UX aspects and trying to break the notebook:

Just a minor note but the waffle image does not show up for either notebook in the launcher, not sure if that is intended.
In windows Shift-click has the same functionality as CRTL-Click
We may want to include instructions of what right clicking does and how to restore the image to the default.
As @JuanCab had noted above if you click repeatedly where there is no star the error message will move the image box. This does reset after 4 messages and the image box reverts back to its initial position.
May want to adjust the wording of the Radius setting to be Aperture Radius or adjust the description of it. It confused me for a little bit.

Overall everything passed, I did try breaking the camera selector some again but it passed with blinding colors.

mwcraig · 2024-05-31T18:38:26Z

@JuanCab @Tanner728 -- I'll open a separate PR today to add the waffle image to the repo.

mwcraig · 2024-05-31T18:38:52Z

We may want to include instructions of what right clicking does and how to restore the image to the default.

Does right-click reset the zoom?

Tanner728 · 2024-05-31T20:54:28Z

We may want to include instructions of what right clicking does and how to restore the image to the default.

Does right-click reset the zoom?

holding right-click and dragging adjusts brightness and contrast of the image depending on which way you drag your cursor while holding right-click. Then I think its just right-click to reset contrast and brightness, it could be shift right-click as well. I forgot.

mwcraig · 2024-05-31T21:08:00Z

@JuanCab -- I think I've addressed all of the review comments except the one about the waffle logo and the one I opened #360 for.

stellarphot/gui_tools/tests/test_seeing_profile.py

JuanCab

Looks good, only suggested edit is my usual request for clearer comments, this time just explaining what a test is testing.

JuanCab · 2024-06-03T19:42:07Z

stellarphot/gui_tools/tests/test_seeing_profile.py

+
+
+def test_seeing_profile_error_messages_no_star(tmp_path):
+    # Make sure the appropriate error message is displayed when a click happens on


Logic for this test seems sound.

mwcraig added 12 commits May 22, 2024 16:05

Add camera chooser to seeing profile

68f3cf1

WIP hack on notebook 01

b5cd823

Add way to programmatically set details visibility

4f8d4ab

Hide camera details by default

ad3d8fa

Provide user feedback if no star found

088fdee

Make save button save settings to photometry file

6e385f0

Give variable more meaningful names

724bb68

Make error display stand out more

49b94b3

Add visual cue that settings need to be changed

ddefd3b

Clean up notebook 1

77938c9

Drop unused code

54b3998

Add launcher entry for notebook 1

6c635b5

mwcraig added the UI 🤮 label May 28, 2024

mwcraig added this to the 2.0.0 -- Reimplemented data structure milestone May 28, 2024

mwcraig requested review from JuanCab and Tanner728 May 28, 2024 14:56

Add camera if it does not exist

d07561d

JuanCab requested changes May 29, 2024

View reviewed changes

Tanner728 approved these changes May 29, 2024

View reviewed changes

mwcraig mentioned this pull request May 31, 2024

Fix description of aperture radius #360

Closed

mwcraig added 4 commits May 31, 2024 14:59

Add test of save method

a146b1d

Add test of whether the save box title is correct

a4bda8c

Limit error message display to one error

82c5b79

Add test of property

a64d47d

mwcraig added 2 commits May 31, 2024 16:55

Skip coverage on an exception

54b8a4a

Use variable names for special characters

e13ddc2

mwcraig requested a review from JuanCab June 3, 2024 15:28

JuanCab reviewed Jun 3, 2024

View reviewed changes

stellarphot/gui_tools/tests/test_seeing_profile.py Show resolved Hide resolved

JuanCab approved these changes Jun 3, 2024

View reviewed changes

mwcraig merged commit 468a836 into feder-observatory:main Jun 3, 2024
11 checks passed

mwcraig deleted the camera-nb-01 branch June 3, 2024 19:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update seeing profile/aperture settings notebook to use new infrastructure #358

Update seeing profile/aperture settings notebook to use new infrastructure #358

mwcraig commented May 28, 2024

JuanCab commented May 28, 2024

mwcraig commented May 29, 2024

codecov bot commented May 29, 2024 •

edited

Loading

JuanCab commented May 29, 2024

JuanCab May 29, 2024

mwcraig May 31, 2024

JuanCab May 29, 2024

mwcraig May 31, 2024

Tanner728 left a comment

mwcraig commented May 31, 2024

mwcraig commented May 31, 2024

Tanner728 commented May 31, 2024 •

edited

Loading

mwcraig commented May 31, 2024

JuanCab left a comment

JuanCab Jun 3, 2024



		def test_seeing_profile_error_messages_no_star(tmp_path):
		# Make sure the appropriate error message is displayed when a click happens on

Update seeing profile/aperture settings notebook to use new infrastructure #358

Update seeing profile/aperture settings notebook to use new infrastructure #358

Conversation

mwcraig commented May 28, 2024

JuanCab commented May 28, 2024

mwcraig commented May 29, 2024

codecov bot commented May 29, 2024 • edited Loading

Codecov Report

JuanCab commented May 29, 2024

JuanCab May 29, 2024

Choose a reason for hiding this comment

mwcraig May 31, 2024

Choose a reason for hiding this comment

JuanCab May 29, 2024

Choose a reason for hiding this comment

mwcraig May 31, 2024

Choose a reason for hiding this comment

Tanner728 left a comment

Choose a reason for hiding this comment

mwcraig commented May 31, 2024

mwcraig commented May 31, 2024

Tanner728 commented May 31, 2024 • edited Loading

mwcraig commented May 31, 2024

JuanCab left a comment

Choose a reason for hiding this comment

JuanCab Jun 3, 2024

Choose a reason for hiding this comment

codecov bot commented May 29, 2024 •

edited

Loading

Tanner728 commented May 31, 2024 •

edited

Loading