Grotrian diagram IPyWidget functionality #2328

AyushiDaksh · 2023-06-15T02:55:40Z

📝 Description

This PR is part of the Grotrian Diagram project, part of Google Summer of Code 2023. Project description and related PRs/Issues can be found on the project page.

This PR aims to integrate the Plotly plot with IPyWidgets menus. In other words, this PR adds functionality to the Grotrian Diagram widget.

Type: 🚀 feature

Also, link issues affected by this pull request by using the keywords: close, closes, closed, fix, fixes, fixed, resolve, resolves or resolved.

📌 Resources

Project Link: https://github.com/users/AyushiDaksh/projects/1
Idea Link: https://tardis-sn.github.io/gsoc_2023/ideas/
GSoC Project Link: https://summerofcode.withgoogle.com/programs/2023/projects/HxymUMRe

🚦 Testing

How did you test these changes?

Testing pipeline
Other method (describe)
My changes can't be tested (explain why)

☑️ Checklist

I requested two reviewers for this pull request
I updated the documentation according to my changes
I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.

review-notebook-app · 2023-06-15T02:55:45Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

codecov · 2023-07-28T08:29:28Z

Codecov Report

Attention: Patch coverage is 12.38095% with 92 lines in your changes are missing coverage. Please review.

Project coverage is 68.89%. Comparing base (5bca272) to head (e6aed1e).

❗ Current head e6aed1e differs from pull request most recent head d71c714. Consider uploading reports for the commit d71c714 to get more accurate results

Files	Patch %	Lines
tardis/visualization/widgets/grotrian.py	12.38%	92 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2328      +/-   ##
==========================================
+ Coverage   67.51%   68.89%   +1.37%     
==========================================
  Files         170      165       -5     
  Lines       14228    14081     -147     
==========================================
+ Hits         9606     9701      +95     
+ Misses       4622     4380     -242

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

MarkMageeAstro

Haven't been able to review the code itself yet, but there are some minor changes to the documentation that should be implemented.

docs/io/visualization/generating_widgets.ipynb

docs/io/visualization/using_widgets.rst

andrewfullard · 2023-09-01T14:19:30Z

Can this be merged?

jamesgillanders · 2023-11-10T15:16:38Z

@MarkMageeAstro @jamesgillanders @aoifeboyle(?) to review/test this and make a decision to merge or update

jamesgillanders · 2024-01-19T15:18:28Z

@AyushiDaksh we are keen to merge this PR -- can you please rebase it?

MarkMageeAstro · 2024-02-02T13:49:37Z

docs/io/visualization/generating_widgets.ipynb

@@ -5,16 +5,17 @@
   "metadata": {},


These links don't work for me. Might be a simple rebasing issue

Reply via ReviewNB

@MarkMageeAstro This is the same link which is present in the latest master branch as well.
https://tardis-sn.github.io/tardis/using/visualization/using_widgets.html

Okay, can you open an issue?

Opened: #2532

@MarkMageeAstro

I've solved the issue in this PR: #2533

docs/io/visualization/generating_widgets.ipynb

MarkMageeAstro

Minor changes to documentation. Should be good to merge after rebasing. Is that Grotrian mockup notebook still necessary?

docs/io/visualization/using_widgets.rst

AyushiDaksh · 2024-02-05T19:00:55Z

@MarkMageeAstro @jamesgillanders
Sorry I somehow saw the comments now. Yes we're keen on merging this.
I'll rebase this branch and make the requested changes today

MarkMageeAstro · 2024-02-14T17:14:15Z

Any luck with this @AyushiDaksh?

MarkMageeAstro · 2024-02-21T09:55:29Z

@AyushiDaksh can you let us know if you're still interested in this?

AyushiDaksh · 2024-03-01T04:40:04Z

@MarkMageeAstro Sorry I moved to a new city this month so the last couple of weeks were really busy.
I have started work on this now, will wrap it up this weekend

AyushiDaksh · 2024-03-01T05:53:54Z

Minor changes to documentation. Should be good to merge after rebasing. Is that Grotrian mockup notebook still necessary?

I have changed the docs according to the comments and rebased the branch.
The mockup notebook helps the user learn to use the tool quickly. I would suggest keeping it.

@MarkMageeAstro @jamesgillanders

tardis-bot · 2024-03-04T14:28:04Z

*beep* *bop*

Hi, human.

The docs workflow has succeeded ✔️

Click here to see your results.

MarkMageeAstro

One minor correction to documentation. Also the mockup notebook didn't show any figures for mean on ReviewNB. Can you generate it again?

Thanks for continuing on this @AyushiDaksh and your hard work getting it done!

AyushiDaksh · 2024-03-08T05:39:44Z

@MarkMageeAstro

One minor correction to documentation.

Somehow I'm not able to see your comment. Can you point me to where you need the correction?

Also the mockup notebook didn't show any figures for mean on ReviewNB. Can you generate it again?

Since the Grotrian Widget is a dynamic widget, it doesn't persist in the notebook when it gets saved. This is the same reason why other dynamic widgets don't show up on our documentation page (and we have to resort to GIFs instead).

docs/io/visualization/using_widgets.rst

MarkMageeAstro · 2024-03-11T11:33:16Z

@MarkMageeAstro

One minor correction to documentation.

Somehow I'm not able to see your comment. Can you point me to where you need the correction?

Hmm, I can't find it either. Must not have been that important. I did spot one other small thing when looking for it thoguh, sorry.

Also the mockup notebook didn't show any figures for mean on ReviewNB. Can you generate it again?

Since the Grotrian Widget is a dynamic widget, it doesn't persist in the notebook when it gets saved. This is the same reason why other dynamic widgets don't show up on our documentation page (and we have to resort to GIFs instead).

Ah, I must have been confused because there were plots in the old version, but that was with the static version of the code

AyushiDaksh · 2024-03-12T00:14:43Z

Hmm, I can't find it either. Must not have been that important. I did spot one other small thing when looking for it thoguh, sorry.

No worries! I have fixed the thing you pointed.

Ah, I must have been confused because there were plots in the old version, but that was with the static version of the code

Yeah we had a static plot during the initial stages of my project, which eventually became dynamic :)

Let me know if this is good to merge @MarkMageeAstro

This reverts commit 23afbef.

This reverts commit 09b453b.

This reverts commit e9acd6a.

AyushiDaksh · 2024-03-12T00:34:34Z

Rebased with master and tested the widget locally again, all seems good.

Please look at #2328 (comment) and let me know if anything else is pending here.

Thanks!

MarkMageeAstro · 2024-03-12T13:56:26Z

Looks good to me and also works on my local install. Thanks @AyushiDaksh!

AyushiDaksh · 2024-03-15T17:21:15Z

Thanks @MarkMageeAstro !

@jamesgillanders Can you please review/approve so we can merge this?

It was great working on this project with TARDIS. Feel free to tag me in future issues/enhacements related to the Grotrian Diagram.

andrewfullard · 2024-04-12T14:15:50Z

Just one conflict to resolve! Probably due to file renames recently.

atharva-2001 added the visualization label Jun 19, 2023

AyushiDaksh force-pushed the grotrian-diagram-ipywidget-support branch from c38c1c7 to 2e2d031 Compare July 28, 2023 08:14

AyushiDaksh force-pushed the grotrian-diagram-ipywidget-support branch from 8bcca5e to 1759798 Compare August 4, 2023 22:44

AyushiDaksh marked this pull request as ready for review August 7, 2023 21:03

AyushiDaksh requested review from jamesgillanders, MarkMageeAstro and aoifeboyle August 7, 2023 21:04

MarkMageeAstro requested changes Aug 11, 2023

View reviewed changes

andrewfullard requested a review from MarkMageeAstro September 18, 2023 15:20

MarkMageeAstro reviewed Feb 2, 2024

View reviewed changes

MarkMageeAstro requested changes Feb 2, 2024

View reviewed changes

docs/io/visualization/using_widgets.rst Outdated Show resolved Hide resolved

docs/io/visualization/using_widgets.rst Outdated Show resolved Hide resolved

AyushiDaksh force-pushed the grotrian-diagram-ipywidget-support branch from 48a571a to 5c5c068 Compare March 1, 2024 05:22

andrewfullard added the build-docs label Mar 4, 2024

andrewfullard requested a review from MarkMageeAstro March 4, 2024 14:19

MarkMageeAstro requested changes Mar 6, 2024

View reviewed changes

MarkMageeAstro reviewed Mar 11, 2024

View reviewed changes

docs/io/visualization/using_widgets.rst Outdated Show resolved Hide resolved

AyushiDaksh and others added 16 commits March 11, 2024 20:29

Remove unnecessary cells from notebook

7b5a0b3

Fixed refresh logic of wavelength slider

40185b2

Update docs for GrotrianDiagram

bd416cc

Refactor code and set default y-coordinate scale to log

7ecd4ad

Updated documentation and minor fixes

d013b90

Minor changes in documentation (in response to review comments)

ea0b25a

Minor change to docs

86afb96

Update the generating_widgets notebook

c2d5090

Minor markdown edit

c97edf6

Rename model to simulation_state

15abc99

Fix paths and Simulation object creation

b19660f

Revert "Fix paths and Simulation object creation"

66d4da7

This reverts commit 23afbef.

Revert "Revert "Fix paths and Simulation object creation""

857d9df

This reverts commit 09b453b.

Render widget in mockup notebook

3444bab

Revert "Render widget in mockup notebook"

8d12084

This reverts commit e9acd6a.

minor doc change

e6aed1e

AyushiDaksh force-pushed the grotrian-diagram-ipywidget-support branch from 16ac28e to e6aed1e Compare March 12, 2024 00:33

andrewfullard requested a review from MarkMageeAstro March 12, 2024 13:30

MarkMageeAstro previously approved these changes Mar 12, 2024

View reviewed changes

Merge branch 'master' into pr/AyushiDaksh/2328

d71c714

andrewfullard dismissed MarkMageeAstro’s stale review via d71c714 April 12, 2024 14:31

jvshields approved these changes Apr 12, 2024

View reviewed changes

andrewfullard enabled auto-merge (squash) April 12, 2024 14:32

andrewfullard approved these changes Apr 12, 2024

View reviewed changes

DeerWhale approved these changes Apr 12, 2024

View reviewed changes

andrewfullard merged commit 28caa4e into tardis-sn:master Apr 12, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Grotrian diagram IPyWidget functionality #2328

Grotrian diagram IPyWidget functionality #2328

AyushiDaksh commented Jun 15, 2023

review-notebook-app bot commented Jun 15, 2023

codecov bot commented Jul 28, 2023 •

edited

Loading

MarkMageeAstro left a comment

andrewfullard commented Sep 1, 2023

jamesgillanders commented Nov 10, 2023

jamesgillanders commented Jan 19, 2024

MarkMageeAstro Feb 2, 2024 •

edited

Loading

AyushiDaksh Mar 1, 2024

MarkMageeAstro Mar 6, 2024

AyushiDaksh Mar 8, 2024

sarthak-dv Mar 8, 2024

MarkMageeAstro left a comment

AyushiDaksh commented Feb 5, 2024

MarkMageeAstro commented Feb 14, 2024

MarkMageeAstro commented Feb 21, 2024

AyushiDaksh commented Mar 1, 2024

AyushiDaksh commented Mar 1, 2024 •

edited

Loading

tardis-bot commented Mar 4, 2024 •

edited

Loading

MarkMageeAstro left a comment

AyushiDaksh commented Mar 8, 2024 •

edited

Loading

MarkMageeAstro commented Mar 11, 2024

AyushiDaksh commented Mar 12, 2024

AyushiDaksh commented Mar 12, 2024 •

edited

Loading

MarkMageeAstro commented Mar 12, 2024

AyushiDaksh commented Mar 15, 2024

andrewfullard commented Apr 12, 2024

Grotrian diagram IPyWidget functionality #2328

Grotrian diagram IPyWidget functionality #2328

Conversation

AyushiDaksh commented Jun 15, 2023

📝 Description

📌 Resources

🚦 Testing

☑️ Checklist

review-notebook-app bot commented Jun 15, 2023

codecov bot commented Jul 28, 2023 • edited Loading

Codecov Report

MarkMageeAstro left a comment

Choose a reason for hiding this comment

andrewfullard commented Sep 1, 2023

jamesgillanders commented Nov 10, 2023

jamesgillanders commented Jan 19, 2024

MarkMageeAstro Feb 2, 2024 • edited Loading

Choose a reason for hiding this comment

AyushiDaksh Mar 1, 2024

Choose a reason for hiding this comment

MarkMageeAstro Mar 6, 2024

Choose a reason for hiding this comment

AyushiDaksh Mar 8, 2024

Choose a reason for hiding this comment

sarthak-dv Mar 8, 2024

Choose a reason for hiding this comment

MarkMageeAstro left a comment

Choose a reason for hiding this comment

AyushiDaksh commented Feb 5, 2024

MarkMageeAstro commented Feb 14, 2024

MarkMageeAstro commented Feb 21, 2024

AyushiDaksh commented Mar 1, 2024

AyushiDaksh commented Mar 1, 2024 • edited Loading

tardis-bot commented Mar 4, 2024 • edited Loading

MarkMageeAstro left a comment

Choose a reason for hiding this comment

AyushiDaksh commented Mar 8, 2024 • edited Loading

MarkMageeAstro commented Mar 11, 2024

AyushiDaksh commented Mar 12, 2024

AyushiDaksh commented Mar 12, 2024 • edited Loading

MarkMageeAstro commented Mar 12, 2024

AyushiDaksh commented Mar 15, 2024

andrewfullard commented Apr 12, 2024

codecov bot commented Jul 28, 2023 •

edited

Loading

MarkMageeAstro Feb 2, 2024 •

edited

Loading

AyushiDaksh commented Mar 1, 2024 •

edited

Loading

tardis-bot commented Mar 4, 2024 •

edited

Loading

AyushiDaksh commented Mar 8, 2024 •

edited

Loading

AyushiDaksh commented Mar 12, 2024 •

edited

Loading