Add tutorial notebook for TIKE #149

rosteen · 2024-10-21T21:20:07Z

I figured I would open a PR here first to get comments, and then we can move/copy the notebook to wherever TIKE needs it for review.

codecov · 2024-10-21T21:23:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.20%. Comparing base (7b78df5) to head (1c83a13).
Report is 43 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #149      +/-   ##
==========================================
- Coverage   93.92%   91.20%   -2.73%     
==========================================
  Files          37       41       +4     
  Lines        1598     2149     +551     
==========================================
+ Hits         1501     1960     +459     
- Misses         97      189      +92

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

notebooks/tike_lcviz_tutorial.ipynb

kecnry · 2024-10-22T16:05:38Z

notebooks/tike_lcviz_tutorial.ipynb

+   "source": [
+    "## Loading light curves into LCviz\n",
+    "\n",
+    "Because TIKE already runs in the cloud, it is fastest and easiest to load data that is also hosted in the cloud using the `s3fs` package, which allows you to access cloud data as if it were local. Here we will use [astroquery](https://astroquery.readthedocs.io/en/latest/) to get the URI for the file that we want to load. For more information about the code below, see [this TIKE webinar notebook](https://github.com/spacetelescope/tike_content/blob/main/content/notebooks/webinar-series/01-lightcurves/01-Lightcurves.ipynb).\n"


since this mentions TIKE specifically, maybe we don't want to actually merge this PR here?

kecnry · 2024-10-22T17:12:50Z

notebooks/tike_lcviz_tutorial.ipynb

+    "light_curve = lightkurve.read(lc_uri)\n",
+    "lcviz.load_data(light_curve)"


@bmorris3 - what would it take to be able to extend the upstream URI capability to be able to pass lc_uri to load_data directly?

It would require one more little block of logic in the jdaviz URI parser. That's work we need to do for RSP soon anyway. In their notebooks, they use this pattern on AWS:

import s3fs import roman_datamodels as rdm asdf_dir_uri = 's3://roman-sci-test-data-prod-summer-beta-test/' fs = s3fs.S3FileSystem() asdf_file_uri = asdf_dir_uri + 'ROMANISIM/DENSE_REGION/R0.5_DP0.5_PA0/r0000101001001001001_01101_0001_WFI16_cal.asdf' with fs.open(asdf_file_uri, 'rb') as f: file = rdm.open(f)

I've tried running the notebook as-is on TIKE and it doesn't work yet, for reasons I don't understand. One challenge that we're likely to face is that lightkurve has tools for initializing a LightCurve from a file path to a FITS file, but not from an open file stream to an HDU list, which is what fsspec is likely to give us.

kecnry · 2024-10-22T17:14:08Z

notebooks/tike_lcviz_tutorial.ipynb

+    "# get the origin of the time axis in LCviz:\n",
+    "time_coordinates = lcviz.app.data_collection[0].coords\n",
+    "reference_time = time_coordinates.reference_time\n",


this isn't technically public API... can this be pulled out of lcviz.get_data(...).meta instead?

Probably, I just pulled this from the LCvizExample notebook. Should probably change it there too if we don't want people using this.

Ok, I'd vote to do that if possible to avoid publicizing any internal API if there are alternatives

kecnry · 2024-10-22T17:15:13Z

notebooks/tike_lcviz_tutorial.ipynb

+    "reference_time = time_coordinates.reference_time\n",
+    "\n",
+    "# literature ephemeris for hot Neptune planet HAT-P-11 b:\n",
+    "morris2017_epoch = Time(2454605.89146, format='jd')\n",


can we avoid using Time entirely or is this needed because reference time is defined differently? Or is it good to show how to do this here?

Again, I just pulled this from the example notebook, which I think @bmorris3 wrote. Probably a question for him.

Why would you want to avoid using Time? Time coordinates are dangerously ambiguous otherwise.

because currently it needs to be passed to lcviz just as a float anyways, so creating a time object just to do to_value seems to add complication to the notebook. Maybe we just link to docs on Time (whether or not we use it), since we don't want this to be a tutorial on that whole topic 🤷‍♂️

I'm working through the notebook, and I think as long as the time system is written in the comment, I'm ok with avoiding astropy.time.Time here. The most likely "gotcha" to worry about is that the units of the reference epoch, the period, and the time axis of the light curve must all be in the same units. If you had period in hours, this wouldn't work correctly. For this example, we're fine without specifying.

kecnry · 2024-10-22T17:15:49Z

notebooks/tike_lcviz_tutorial.ipynb

+    "\n",
+    "- [LCviz documentation](https://lcviz.readthedocs.io/en/stable/index.html)\n",
+    "- [LCviz Github](https://github.com/spacetelescope/lcviz)\n",
+    "- [lightkurve documentation](https://lightkurve.github.io/lightkurve/)\n",


not sure if this link will break in the future when they fix docs.lightkurve.org 🤔

notebooks/tike_lcviz_tutorial.ipynb

Co-authored-by: Kyle Conroy <kyleconroy@gmail.com>

bmorris3

Pending support from Thomas to make the file loading work, this looks good!

notebooks/tike_lcviz_tutorial.ipynb

bmorris3 · 2024-10-28T18:56:37Z

notebooks/tike_lcviz_tutorial.ipynb

+    "reference_time = time_coordinates.reference_time\n",
+    "\n",
+    "# literature ephemeris for hot Neptune planet HAT-P-11 b:\n",
+    "morris2017_epoch = Time(2454605.89146, format='jd')\n",


I'm working through the notebook, and I think as long as the time system is written in the comment, I'm ok with avoiding astropy.time.Time here. The most likely "gotcha" to worry about is that the units of the reference epoch, the period, and the time axis of the light curve must all be in the same units. If you had period in hours, this wouldn't work correctly. For this example, we're fine without specifying.

notebooks/tike_lcviz_tutorial.ipynb

bmorris3 · 2024-10-28T19:00:32Z

notebooks/tike_lcviz_tutorial.ipynb

+    "* [Citing `astropy`](https://www.astropy.org/acknowledging.html)\n",
+    "* [Citing `lightkurve`](http://docs.lightkurve.org/about/citing.html)\n",
+    "\n",
+    "And cite Jdaviz through its [Zenodo record](https://doi.org/10.5281/zenodo.5513927).\n"


@kecnry – should we make a zenodo record for lcviz? (I vote yes!)

yes, let's plan to do that with 1.0 release. I'll add it to the ticket.

bmorris3 · 2024-10-28T19:04:33Z

notebooks/tike_lcviz_tutorial.ipynb

+    }
+   },
+   "source": [
+    "## Imports\n",


lcviz isn't currently installed on the production version of TIKE – I'm assuming that will be done when this notebook is available?

Also, these imports only work if you're in the kernel named TESS Environment. Should we mention that here?

Correct, they will install it once 1.0 is released AND they have this notebook. I think the general instructions handle selecting the kernel environment, but that's a good question for @ttdu

lcviz will be installed in ~December, when we release a new environment for AAS.

we normally specify the default kernel in the notebook metadata, so that TESS Environment is automatically selected. it never hurts to be verbose, but Kyle is correct that the general instructions cover this

bmorris3 · 2024-10-28T19:39:45Z

notebooks/tike_lcviz_tutorial.ipynb

+    "light_curve = lightkurve.read(lc_uri)\n",
+    "lcviz.load_data(light_curve)"


I've tried running the notebook as-is on TIKE and it doesn't work yet, for reasons I don't understand. One challenge that we're likely to face is that lightkurve has tools for initializing a LightCurve from a file path to a FITS file, but not from an open file stream to an HDU list, which is what fsspec is likely to give us.

Co-authored-by: Brett M. Morris <morrisbrettm@gmail.com>

rosteen added 3 commits October 21, 2024 16:43

Add TIKE tutorial notebook for LCviz

81213f9

More updates

16e51f1

Remove template text

199b0f9

rosteen added the documentation Improvements or additions to documentation label Oct 21, 2024

rosteen added the no-changelog-entry-needed label Oct 22, 2024

rosteen added this to the 1.0.0 milestone Oct 22, 2024

kecnry reviewed Oct 22, 2024

View reviewed changes

rosteen and others added 4 commits October 24, 2024 09:54

Update notebooks/tike_lcviz_tutorial.ipynb

5e57f5e

Co-authored-by: Kyle Conroy <kyleconroy@gmail.com>

Update notebooks/tike_lcviz_tutorial.ipynb

f64f33f

Co-authored-by: Kyle Conroy <kyleconroy@gmail.com>

Apply suggestions from code review

776e0b2

Co-authored-by: Kyle Conroy <kyleconroy@gmail.com>

Kepler link instead of TESS

6f6f7e8

bmorris3 approved these changes Oct 28, 2024

View reviewed changes

rosteen and others added 2 commits October 28, 2024 16:41

Apply suggestions from code review

abd0ab8

Co-authored-by: Brett M. Morris <morrisbrettm@gmail.com>

Use public API to get time origin

1c83a13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tutorial notebook for TIKE #149

Add tutorial notebook for TIKE #149

rosteen commented Oct 21, 2024

codecov bot commented Oct 21, 2024 •

edited

Loading

kecnry Oct 22, 2024

kecnry Oct 22, 2024

bmorris3 Oct 25, 2024

bmorris3 Oct 28, 2024

kecnry Oct 22, 2024

rosteen Oct 24, 2024

kecnry Oct 28, 2024

kecnry Oct 22, 2024

rosteen Oct 24, 2024

bmorris3 Oct 25, 2024

kecnry Oct 25, 2024

bmorris3 Oct 28, 2024

kecnry Oct 22, 2024

bmorris3 left a comment

bmorris3 Oct 28, 2024

bmorris3 Oct 28, 2024

kecnry Oct 28, 2024

bmorris3 Oct 28, 2024

kecnry Oct 28, 2024

ttdu Oct 28, 2024

bmorris3 Oct 28, 2024

		"light_curve = lightkurve.read(lc_uri)\n",
		"lcviz.load_data(light_curve)"

Add tutorial notebook for TIKE #149

Are you sure you want to change the base?

Add tutorial notebook for TIKE #149

Conversation

rosteen commented Oct 21, 2024

codecov bot commented Oct 21, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmorris3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 21, 2024 •

edited

Loading