Add a widget for settings like Camera that can be saved with names #294

mwcraig · 2024-04-26T22:05:28Z

This PR has a few things:

Adds a couple of custom widgets to support saving settings like camera that have names.
Tests for those widgets.
A notebook to use for testing out the widget, integrated into the jupyterlab launcher.

In addition to the code review please try going through the notebook. Once you have done a pip install -e . with this branch in your environment, you should see the button in the launcher:

@JuanCab -- if you could review by early next week that would be great.

codecov-commenter · 2024-04-26T22:12:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.70%. Comparing base (f927559) to head (f960057).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #294      +/-   ##
==========================================
+ Coverage   71.86%   73.70%   +1.83%     
==========================================
  Files          26       27       +1     
  Lines        3220     3388     +168     
==========================================
+ Hits         2314     2497     +183     
+ Misses        906      891      -15

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

mwcraig · 2024-04-27T16:48:09Z

@JuanCab -- it may be easiest to review these changes in commit order now. The first few commits are small, and made to lay the groundwork for the new custom widgets.

The commit to add the custom widget and the commit to add its tests are pretty chunky...

mwcraig · 2024-04-27T17:02:50Z

Instructions for student testers

make a new environment, for example: mamba create -n test-294 python=3.11 astroscrappy
activate the new environment: mamba activate test-294
install the version of stellarphot in this PR: pip install git+https://github.com/mwcraig/stellarphot.git@add-jup-app-launcher
Start jupyter lab: jupyter lab
Open the testing notebook in the "Stellarphot" section of the launcher
Email feedback to @mwcraig or open a new issue in this repo with your feedback.
Once you quit jupyter lab, deactivate the environment: mamba deactivate
Delete the test environment: mamba remove --all --yes test-294

mwcraig · 2024-04-27T22:17:59Z

A few things that @Tanner728 found:

You can type stuff in nullable fields (e.g. AAVSO code for observatory) even if the "switch" is off if you have hit the "Edit" button
"Edit" button appears if there are no items in a group (e.g. no observatories) and it shouldn't.
Might be possible to enter an invalid name (e.g. trailing space) in the form.
The edit button doesn't appear after making a new item.

There were a couple other things I'm opening issues for.

JuanCab

Changes for deleting items code makes sense AND the tests seem to cover all the possibilities I can think of.

JuanCab

See notes about the documentation, the actual code seems sound.

stellarphot/settings/models.py

JuanCab · 2024-04-28T18:29:58Z

@JuanCab -- it may be easiest to review these changes in commit order now. The first few commits are small, and made to lay the groundwork for the new custom widgets.

The commit to add the custom widget and the commit to add its tests are pretty chunky...

I didn't know I could go commit by commit, doing that now. It is easier!

JuanCab

Documentation tweaks requested... looks like a very challenging customization, you did a lot here!

JuanCab · 2024-04-28T18:32:22Z

stellarphot/settings/custom_widgets.py

+        Name of the item type to be displayed in the widget. Must be one of
+        "camera", "observatory", "passband_map", "Camera", "Observatory"
+        or "PassbandMap".
+    """


Documentation is missing the keyword _testing_path... is that meant to eventually go away?

I wasn't sure how to handle this...the intent is that users don't use it, it is just there for testing. I could hide it more, I suppose, by putting it into **kwargs and popping it from there. That way it wouldn't show up in code completion.

I suppose parameters can't be flagged as private (correction, private doesn't exist really in Python, I was thinking of name mangling via double underscore, but that won't help here). But it makes sense that it is for testing purposes.

JuanCab · 2024-04-28T18:35:50Z

stellarphot/settings/custom_widgets.py

+            # but does no harm in the other cases (true of both lines below)
+            # (and yes, both lines below are needed...this is a bug in ipyautoui,
+            #  I think, because open_nested=True isn't respected when we _init_ui.
+            #  Forcing a *change* in the value triggers the behavior we want.)


Cute fix for the bug.

JuanCab · 2024-04-28T18:37:08Z

stellarphot/settings/custom_widgets.py

+        # but does no harm in the other cases (true of both lines below)
+        # (and yes, both lines below are needed...this is a bug in ipyautoui,
+        #  I think, because open_nested=True isn't respected when we _init_ui.
+        #  Forcing a *change* in the value triggers the behavior we want.)


Given you have the same comment and fix twice, makes me wonder if you should just create a function called _jiggle_open_nested_for_ipyautoui. :)

JuanCab · 2024-04-28T18:42:11Z

stellarphot/settings/custom_widgets.py

+
+        # Use a closure here to capture the current state of the widget
+        def save_confirm_handler(change):
+            # value of None means the widget has been reset to not answered


Do you want to clarify this is that the "confirmation" has not been addressed or the "confirmation widget" has been reset to behave as if not answered? I was confused by this at first and only realized what you meant by reading the behavior below. Better yet, maybe place a comment in the function 'docstring' above explaining what the desired behavior is as a whole.

JuanCab

Looks good, I didn't know how you could check UI features, now I know. Made sense, although I had to concentrate at a few points to follow it.

JuanCab · 2024-04-28T18:51:47Z

pyproject.toml

-    'ignore:Using extra keyword arguments on `Field` is deprecated:'
+    'ignore:Using extra keyword arguments on `Field` is deprecated:',
+    # ipywidgets is using something deprecated in traitlets
+    'ignore:Deprecated in traitlets 4.1, use the instance:DeprecationWarning',


So we have to wait for an ipywidgets update to address this...

JuanCab

I tweaked my stellardev.yml file for the stellarphot development environment to include the Jupyter app launcher.

JuanCab · 2024-04-28T19:20:35Z

All looks good code wise, I had a few documentation issues, but they were minor. I did also try out the notebooks, using the instructions to students above (after editing them to replace a test-394 with test-294).

For the camera, it keeps displaying "(red X) Invalid" at the top, same with Passband Map (but not Observatory). I can't set a Camera name, which may be the problem.... it's greyed out. The Passband Map name is not filled out but I can't seem to get it to be editable. Rebuilt the widget from scratch by re-running the cell that has widgets = [ChooseOrMakeNew(item) for item in to_make] tp create the widgets.. then understood a few issues.

Some testing reveals if I forget to fill out the Bandpass Map NAME before trying to actually add a mapping, the Name becomes uneditable, which condemns me to an invalid Bandpass Map.

For the Camera, I was not able to replicate the problem in some limited testing, the second time, choosing the values that were default did work.

JuanCab · 2024-04-28T19:21:25Z

I do not get the sparkly waffle icon BTW, I just get this:

mwcraig · 2024-04-29T15:37:27Z

I'm going merge this -- I addressed the documentation comments, I think, and have opened issues for the rest.

mwcraig added this to the 2.0.0 -- Reimplemented data structure milestone Apr 26, 2024

mwcraig requested a review from JuanCab April 26, 2024 22:05

mwcraig force-pushed the add-jup-app-launcher branch 2 times, most recently from 8e220c0 to af067b8 Compare April 27, 2024 16:16

mwcraig added 7 commits April 27, 2024 11:46

Add ability to delete saved setting and an option for case matching

85a7914

Update PassbandMap model

8b71a55

Update ui_generator

11dde60

Add custom widgets for named settings

05bf048

Add tests for custom widgets

bcd91c7

Ignore more deprecation warnings

0255709

Add basic app launcher configuration notebook for testing

5d0d88f

mwcraig force-pushed the add-jup-app-launcher branch from af067b8 to 5d0d88f Compare April 27, 2024 16:46

mwcraig changed the title ~~Add a widget for settings linke Camera that can be saved with names~~ Add a widget for settings like Camera that can be saved with names Apr 27, 2024

JuanCab approved these changes Apr 28, 2024

View reviewed changes

JuanCab requested changes Apr 28, 2024

View reviewed changes

stellarphot/settings/models.py Show resolved Hide resolved

JuanCab approved these changes Apr 28, 2024

View reviewed changes

JuanCab reviewed Apr 28, 2024

View reviewed changes

JuanCab approved these changes Apr 28, 2024

View reviewed changes

JuanCab previously approved these changes Apr 28, 2024

View reviewed changes

This was referenced Apr 28, 2024

Name entry locks after making edits in other fields #301

Closed

Make sure the logo is packaged #302

Closed

mwcraig added 2 commits April 28, 2024 15:53

Fix a couple of typos

c4f05d9

Add more explanation to confirmation handler

f960057

mwcraig dismissed JuanCab’s stale review via f960057 April 28, 2024 20:53

mwcraig merged commit a111b68 into feder-observatory:main Apr 29, 2024
9 checks passed

mwcraig deleted the add-jup-app-launcher branch April 29, 2024 15:38

mwcraig restored the add-jup-app-launcher branch April 29, 2024 15:38

mwcraig deleted the add-jup-app-launcher branch May 5, 2024 17:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a widget for settings like Camera that can be saved with names #294

Add a widget for settings like Camera that can be saved with names #294

mwcraig commented Apr 26, 2024

codecov-commenter commented Apr 26, 2024 •

edited

mwcraig commented Apr 27, 2024

mwcraig commented Apr 27, 2024 •

edited by JuanCab

mwcraig commented Apr 27, 2024 •

edited

JuanCab left a comment

JuanCab left a comment

JuanCab commented Apr 28, 2024

JuanCab left a comment

JuanCab Apr 28, 2024

mwcraig Apr 28, 2024

JuanCab Apr 28, 2024 •

edited

JuanCab Apr 28, 2024

JuanCab Apr 28, 2024

JuanCab Apr 28, 2024

JuanCab left a comment

JuanCab Apr 28, 2024

mwcraig Apr 28, 2024

JuanCab left a comment

JuanCab commented Apr 28, 2024

JuanCab commented Apr 28, 2024

mwcraig commented Apr 29, 2024

Add a widget for settings like Camera that can be saved with names #294

Add a widget for settings like Camera that can be saved with names #294

Conversation

mwcraig commented Apr 26, 2024

codecov-commenter commented Apr 26, 2024 • edited

Codecov Report

mwcraig commented Apr 27, 2024

mwcraig commented Apr 27, 2024 • edited by JuanCab

Instructions for student testers

mwcraig commented Apr 27, 2024 • edited

JuanCab left a comment

Choose a reason for hiding this comment

JuanCab left a comment

Choose a reason for hiding this comment

JuanCab commented Apr 28, 2024

JuanCab left a comment

Choose a reason for hiding this comment

JuanCab Apr 28, 2024

Choose a reason for hiding this comment

mwcraig Apr 28, 2024

Choose a reason for hiding this comment

JuanCab Apr 28, 2024 • edited

Choose a reason for hiding this comment

JuanCab Apr 28, 2024

Choose a reason for hiding this comment

JuanCab Apr 28, 2024

Choose a reason for hiding this comment

JuanCab Apr 28, 2024

Choose a reason for hiding this comment

JuanCab left a comment

Choose a reason for hiding this comment

JuanCab Apr 28, 2024

Choose a reason for hiding this comment

mwcraig Apr 28, 2024

Choose a reason for hiding this comment

JuanCab left a comment

Choose a reason for hiding this comment

JuanCab commented Apr 28, 2024

JuanCab commented Apr 28, 2024

mwcraig commented Apr 29, 2024

codecov-commenter commented Apr 26, 2024 •

edited

mwcraig commented Apr 27, 2024 •

edited by JuanCab

mwcraig commented Apr 27, 2024 •

edited

JuanCab Apr 28, 2024 •

edited