Add a review widget

[mwcraig]

This pull request adds what will be the first of the new photometry notebooks (setting camera, observatory, passband map) and the final review-of-settings notebook.

Remaining to be done:

• [x] add unit tests for ReviewSettings
• [x] add notebook for first step
• [x] add notebook for last step
• [x] add launcher entries for notebooks

Fixes #377 which was uncovered during development of this new widget. Fixes #182 -- the notebooks were not combined but things have been streamlined a bit.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.44%. Comparing base (292e181) to head (4867a3b). Report is 8 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #371 +/- ## ========================================== + Coverage 77.66% 78.44% +0.78% ========================================== Files 27 27 Lines 3626 3758 +132 ========================================== + Hits 2816 2948 +132 Misses 810 810 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[mwcraig]

Note: the linting failure is because ruff changed its rules -- it is not due to a code change. I'm planning to ignore it for now and open an issue to fix it later.

[mwcraig]

@Tanner728 -- please try breaking the two new notebooks, called "1 - Saved settings" and "4 - Review settings".

I would try these in a new folder because they will save files to that folder.

These new notebooks should

• Automatically save settings to either partial_photometry_settings.json or photometry_setttings.json depending on whether you have a complete set of settings or not.
• Initially each tab will be labeled either with "🔆" meaning the settings in that tab have been save but should be reviewed or "❌" indicating the setting has not been saved. When you click on a "🔆" tab the mark will typically change to "✅" indicating both that you have checked the setting and it has been saved to the local working directory.
• Selecting a new different camera or observatory or passband map should result in a green check and that camera saved to the local directory.
• Editing values in any of the widgets should change things to a red X until you hit the save button.

Please don't break it too hard 😬

[Tanner728]

Here are the notes of what I have found from testing:

• The initial "SBIG FakeCam" example is invalid. If you select "Edit this Camera" you will not be able to save the camera. That is caused by the Dark Current initially being expressed as "0.01 electron / s". The code appears to need "second" written out.

• "Name" field still checks for blank space at the end of the name.

• Widget still works with foreign languages, I only tested Korean this time.

• The Filter Wheel tab allows for identical naming of different filters. Not sure if this is something we need to address or leave as is.

• Values in the settings JSON correctly update when a new camera or filter wheel are selected.

@mwcraig I am unable to launch Notebook 3 and Notebook 4 at this time. I have tried restarting the kernel and closing everything as well as restarting Jupyter. Not sure what is going on as I do not get an error when clicking on Notebooks 3 and 4, just nothing happens. Would you be able to confirm that they launch for you? Or do I need to progress through the notebooks sequentially?

[mwcraig]

@Tanner728 the notebooks launched earlier today (or at least I think so). You could download the notebooks from GitHub and they should work. I'll double check in the morning.

[mwcraig]

@Tanner728 -- I just pushed a fix for the notebook 3 and 4 issues. Thanks for catching that!

[mwcraig]

Oops, just actually pushed the notebook fix

[Tanner728]

@mwcraig I will most likely not have a chance to review the notebooks until this weekend or early next week. But I will let you know when I break more things or fail to break things. :)

[mwcraig]

@Tanner728 -- thanks for the update. I may end up merging this by then, but the breaking could always come later. I'd really, really, like to get even a rough 2.0 out before scipy next week.

[Tanner728]

@mwcraig I can do a quick breaking run either tomorrow or friday then to catch any big issues before Scipy.

[Tanner728]

Hello It looks like I will not have a chance to check these notebooks until after the weekend.

[JuanCab]

This might be better as a new issue than as something to do for this PR to get merged...

I used the review widget, I noticed that while we got an indication the whole model was bad, there was no way to find out WHY the model was bad. I purposely mis-entered the units for one entry and there was no clear indication that was the problem.

From what I understand of ipyautoui, we are suppressing the actual validation error message (which makes sense). But would it be possible to have a way of displaying the validation error if the user so chooses? Or if you want to get clever you could parse the error message and visually flag the individual bad entries, although that seems like a lot more hard work.

[JuanCab]

All the following tests were done in Jupyter Lab, I did test a little with VS Code and it seems consistent except some of the drop downs do NOT render properly. That might be an issue with how iPyAutoui creates its custom widgets...

Other glitches I noticed:

• The dropdown menus don't seem to work in expected ways sometimes: Specifically:

  • Case 1: Repeated options

    1. Run all cells
    2. Select make a new camera (I used ` from drop down).
    3. Select adu for data units and 1.5 electrons / adu for gain
    4. Glitch? Read noise now presents three identical copies of 10.0 electron for noise.
    5. Glitch? Dark current now presents three identical copies of 0.01 electron / sec for noise.
    6. Glitch? I was presented with the option to select various DN, including two with the wrong, invalid units given all the previous settings. Not necessarily easily avoidable, but I could imagine it as a bit confusing for first-timers at photometry.

  • Case 2: No Option

    1. Run all cells
    2. Select to edit the Testing Camera
    3. Go to the dropdown menu for Gain... it has no options. Same with Max Data Value.

• Another issue: AAVSO filters appear twice in dropdown

  1. I opted to edit the built in Filter wheel 1 under the Passband map tab.
  2. I clicked the red + button
  3. I added 'r' as a hypothetical filter and selected the dropdown... the R and I filters were listed twice

[Tanner728]

Reviewing this is on my to-do list for tonight. Just FYI @mwcraig

[mwcraig]

Case 1: Repeated options .... Glitch? Read noise now presents three identical copies of 10.0 electron for noise.

Can you double check? The last option is still 10.0 photon for me.

Glitch? Dark current now presents three identical copies of 0.01 electron / sec for noise.

Same here, I'm seeing 0.01 photon / sec as the last option.

Glitch? I was presented with the option to select various DN, including two with the wrong, invalid units given all the previous settings. Not necessarily easily avoidable, but I could imagine it as a bit confusing for first-timers at photometry.

More a design feature, though maybe it ends up being confusing. Each dropdown has three options. The set of first options are consistent with each other, the set of second options are consistent and the set of third options are consistent. However, there is no updating of the suggestions as fields are filled out. That could maybe be added iun the future...

[mwcraig]

Case 2: No Option

Run all cells Select to edit the Testing Camera Go to the dropdown menu for Gain... it has no options. Same with Max Data Value.

This is because by default ipyautoui makes an ipywidgets.Combobox which is a combination text box and dropdown. The dropdowns are only suggestions; any value is allowed.

In addition, the dropdowns are only shown if there is no text in the box.

You can see the same behavior in the ipywidgets documentation I linked to above.

[mwcraig]

Another issue: AAVSO filters appear twice in dropdown ... I added 'r' as a hypothetical filter and selected the dropdown... the R and I filters were listed twice

Ah, that is because the AAVSO effectively has its own passsband map which maps R and Rc to R and similar for I, so each shows up twice.

Will open a separate issue for this.

[mwcraig]

Another issue: AAVSO filters appear twice in dropdown ... I added 'r' as a hypothetical filter and selected the dropdown... the R and I filters were listed twice

Ah, that is because the AAVSO effectively has its own passsband map which maps R and Rc to R and similar for I, so each shows up twice.

Will open a separate issue for this.

[mwcraig]

This might be better as a new issue than as something to do for this PR to get merged...

I used the review widget, I noticed that while we got an indication the whole model was bad, there was no way to find out WHY the model was bad. I purposely mis-entered the units for one entry and there was no clear indication that was the problem.

From what I understand of ipyautoui, we are suppressing the actual validation error message (which makes sense). But would it be possible to have a way of displaying the validation error if the user so chooses?

This is essentially the idea in: https://github.com/maxfordham/ipyautoui/issues/308

For now, can you open an issue in this repo, @JuanCab, to add an option to display the errors, maybe by clicking on the red x?

[JuanCab]

Case 1: Repeated options .... Glitch? Read noise now presents three identical copies of 10.0 electron for noise.

Can you double check? The last option is still 10.0 photon for me.

Glitch? Dark current now presents three identical copies of 0.01 electron / sec for noise.

Same here, I'm seeing 0.01 photon / sec as the last option.

Here are screen shots showing the menus I am seeing:

[mwcraig]

Here are screen shots showing the menus I am seeing:

Ooooohhhhh, for a moment I was an old man shaking his fist in the sky in frustration that something was broken, but now I think I know what is going on.

What I said before was not quite right:

In addition, the dropdowns are only shown if there is no text in the [combo] box.

What is displayed in the combobox is only the options consistent with the text entered so far, so when 10.0 electron is already in the read noise, the combobox only shows the two entries consistent with that. If you were to erase the contents of that box you would see three choices, the last of which would be 10.0 photon.

I'm not sure what to do about this....

[Tanner728]

I am starting to review this and just had a quick question. Do we need to provide an input box for the focal length of the system being used? I know we have our FL in the FITS header but does the "Sky" coordinate system, which I assume is WCS, need a focal length to plate solve the images?

[mwcraig]

. Do we need to provide an input box for the focal length of the system being used? I know we have our FL in the FITS header but does the "Sky" coordinate system, which I assume is WCS, need a focal length to plate solve the images?

I don't think so -- the FL can be calculated from some of the parameters that the plate solve generates. I think it could also be used as an input, but we haven't done it that way.

I gather, though, that that is something you need to provide as an input for your plate solves?

[Tanner728]

Yeah, from my experience using Pixinsight and ASTAP (standalone platesolver) they need a focal length. It doenst need to be exact but as long as it is within putting distance of the actual FL it works.

[mwcraig]

I think we don't need that here because I've been assuming images are already plate solved....

[mwcraig]

Looks good, two typo suggestions made, but otherwise, good to go.

Merging once tests pass 😬 🎉