Update example notebooks to use in-flight JWST data #1680
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -23,8 +23,9 @@ | |||||
| "outputs": [], | ||||||
| "source": [ | ||||||
| "import warnings\n", | ||||||
| "import tempfile\n", | ||||||
| "\n", | ||||||
| "from astroquery.mast import Observations\n", | ||||||
| "from photutils import CircularAperture\n", | ||||||
| "from regions import PixCoord, CirclePixelRegion\n", | ||||||
| "\n", | ||||||
|
|
@@ -41,7 +42,9 @@ | |||||
| { | ||||||
| "cell_type": "code", | ||||||
| "execution_count": null, | ||||||
| "metadata": { | ||||||
| "scrolled": false | ||||||
| }, | ||||||
| "outputs": [], | ||||||
| "source": [ | ||||||
| "cubeviz = Cubeviz()\n", | ||||||
|
|
@@ -52,7 +55,17 @@ | |||||
| "cell_type": "markdown", | ||||||
| "metadata": {}, | ||||||
| "source": [ | ||||||
| "Here we download a JWST MIRI cube from [MAST](https://masttest.stsci.edu/) using `astroquery`, \n", | ||||||
| "to use as our example data. By default the downloaded file goes to your\n", | ||||||
| "temp directory, and thus may eventually be garbage collected (deleted) by your system.\n", | ||||||
| "If you would like to have the file permanently, simply uncomment the second line below\n", | ||||||
| "and provide the directory path on your system where you would like the file stored.\n", | ||||||
| "\n", | ||||||
| "One other thing to note about retrieving MAST data through astroquery is that it caches\n", | ||||||
| "the data by default. It is possible for files to be updated in MAST with more recent calibrations\n", | ||||||
| "but remain the same size, in which case your local cached file would not be replaced by the new\n", | ||||||
| "version. You can turn off caching by setting `cache=False` in the `download_file` call to\n", | ||||||
| "force it to re-download the file if desired." | ||||||
| ] | ||||||
| }, | ||||||
| { | ||||||
|
|
@@ -61,11 +74,15 @@ | |||||
| "metadata": {}, | ||||||
| "outputs": [], | ||||||
| "source": [ | ||||||
| "data_dir = tempfile.gettempdir()\n", | ||||||
| "#data_dir = \"/User/username/Data/\"\n", | ||||||
| "\n", | ||||||
| "fn = \"jw02732-c1001_t004_miri_ch1-longmediumshort-_s3d.fits\"\n", | ||||||
| "result = Observations.download_file(f\"mast:JWST/product/{fn}\", local_path=f'{data_dir}/{fn}')\n", | ||||||
|
There was a problem hiding this comment. You should not have to include filename in the path?
Suggested change
There was a problem hiding this comment. I agree that you shouldn't, but when I tested without it failed (contrary to the There was a problem hiding this comment. Oh, a known bug: astropy/astroquery#2501 There was a problem hiding this comment. yes I think it's supposed to be a path rather than a filename. |
||||||
| "\n", | ||||||
| "with warnings.catch_warnings():\n", | ||||||
| " warnings.simplefilter('ignore')\n", | ||||||
| " cubeviz.load_data(f'{data_dir}/{fn}')" | ||||||
| ] | ||||||
| }, | ||||||
| { | ||||||
|
|
@@ -183,13 +200,6 @@ | |||||
| "subset1_spec1d = spectra_dict['Subset 1']\n", | ||||||
| "subset1_spec1d" | ||||||
| ] | ||||||
| } | ||||||
| ], | ||||||
| "metadata": { | ||||||
|
|
@@ -208,7 +218,7 @@ | |||||
| "name": "python", | ||||||
| "nbconvert_exporter": "python", | ||||||
| "pygments_lexer": "ipython3", | ||||||
| "version": "3.10.6" | ||||||
| } | ||||||
| }, | ||||||
| "nbformat": 4, | ||||||
|
|
||||||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still not a big fan but it's a good compromise until we have the caching debate later. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to cache by default, but at least we can get this in now and have the discussion on the HOW to cache not block this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think right now, by default it won't redownload if it detects the file is already there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cache=Trueby default indownload_file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, right. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would adding a note saying to add
cache=Falseto force a download be helpful here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a double take when I saw @jdavies-st in my morning email digest! Good to see ya :)
I don't think this is a major concern for these example notebooks since these notebooks are just... well examples. I'd hope people aren't taking these notebooks and doing real science on them!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😬 🙊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of a note in the notebook, though, in case someone copies the logic for their own data from MAST and might not understand this... feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually a problem of
astroquery, not specific to MAST. Basically to see if the file it wants to download is cached, it checks the size. It does not do a checksum or hash. But with FITS, two different files can have the same size, and we've actually seen precisely this for JWST _uncal.fits data (and some other data products), which have quite different calibrations. All FITS header cards are the same size, even if the content of the keyword is different.Of course there's big overhead to doing checksums, and that's probably a good enough reason as any for astroquery to not being doing it routinely. But just something to be aware of. =)