[WIP] photutils tutorial notebooks #101

laurenmarietta · 2019-05-20T20:46:25Z

As has been discussed with @eteq, @mwcraig, @kelle, this PR includes four notebooks and supplemental materials for the photutils tutorial that I had been developing on eteq/csi-stsci-notebooks. We determined it makes more sense for them to exist in this repo as we look towards future plans.

As far as I understand them, those future plans include integration with learn.astropy.org (along with the rest of spacetelescope/notebooks), and ultimately being hosted within the photutils documentation.

The four notebooks are ordinal, and cover the following topics:

Background estimation
Source detection
Aperture photometry
PSF photometry (this notebook is currently just a scaffold)

I invite anyone who is curious to review these notebooks now, though I hope to get more content into the PSF photometry notebook before merging.

astro-josh · 2019-05-21T13:39:20Z

Seems it ran into the same execution error on a few of the notebooks:
01_background_estimation
02_source_detection
03_aperture_photometry
04_psf_photometry

[Errno 2] No such file or directory: 'latex': 'latex'

cbar.set_label(r'Flux Count Rate ({})'.format(xdf_image.unit.to_string('latex')), 
               rotation=270, labelpad=30)
ax1.set_xlabel('X (pixels)')
ax1.set_ylabel('Y (pixels)')

laurenmarietta · 2019-05-21T14:30:40Z

I did see that... though I don't get this error when I run the notebooks locally.

Do you have any tips for what I can do to further investigate that and fix it? I am still pretty unfamiliar with exactly how the notebook HTML is compiled.

astro-josh · 2019-05-21T16:23:36Z

I removed the text.usetex line from the mplstyle file and it seems to have done the trick. I believe its just the circleCI linux image missing the required latex formatting resources?

If this is needed, we can try adding sudo apt-get install dvipng texlive-latex-extra texlive-fonts-recommended to the circle config to install what I believe is missing for text.usetex to work.

Here is the latest generated html after removing usetex, please look it over to see if it looks correct and the formulas are formatted as they should be. If not, I can try adding tex to the build machine.

astro-josh

Code/notebook looks great to me! May need someone with a science background to review as well.

astro-josh · 2019-05-22T14:11:15Z

notebooks/photutils/04_psf_photometry/04_psf_photometry.ipynb

+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "## Circular Apertures"


These blank sections are still a WIP correct?

mwcraig

These are really helpful. There are a number of detailed comments, almost all of them minor. It might be worth moving some of the common plotting code out of the notebooks into a single file from which you import it, though the only thing that seemed really repetitious was the colorbar code, which is identical for all of the colorbars.

You should add a minimum version for astropy (3.1, I think) because in some places you use the maxiters keyword argument to sigma_clipped_stats and that was introduced in 3.1. Alternatively, use iters even though it has been deprecated.

I think in notebook 01 you are using iters instead of maxiters; either is fine with the astropy version constraint but should probably be consistent across the notebooks.

notebooks/photutils/01_background_estimation/01_background_estimation.ipynb

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

…tutils # Conflicts: # notebooks/.gitignore

laurenmarietta · 2019-07-31T20:13:37Z

Okay, at this point I have gone through all of @mwcraig's edits that I am capable of responding to. I'm happy to add more changes if something arises in the comments for those I was not able to respond to.

mwcraig · 2019-08-02T14:48:45Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+    "\n",
+    "In the [background estimation notebook](../01_background_estimation/01_background_estimation.ipynb), we explored how to perform global background subtraction of image data with `photutils`. However, you can also use `photutils` to perform local background estimations for aperture corrections.\n",
+    "\n",
+    "To estimate the local background for each aperture, measure the counts within annulus apertures around (but not including!) each source. In our example, we defined elliptical apertures with `r = 3` pixels to measure the counts within each source. To calculate the background for each source, let's measure the counts elliptical annuli between `r = 3.5` pixels and `r = 5` pixels."


Suggested change

"To estimate the local background for each aperture, measure the counts within annulus apertures around (but not including!) each source. In our example, we defined elliptical apertures with `r = 3` pixels to measure the counts within each source. To calculate the background for each source, let's measure the counts elliptical annuli between `r = 3.5` pixels and `r = 5` pixels."

"To estimate the local background for each aperture, measure the counts within annulus apertures around (but not including!) each source. In our example, we defined elliptical apertures with `r = 3` pixels to measure the counts within each source. To calculate the background for each source, let's measure the counts elliptical annuli between `r = 6` pixels and `r = 10` pixels."

mwcraig · 2019-08-02T14:49:18Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "r_in = 3.5  # approximate isophotal extent of inner semimajor axis\n",


Suggested change

"r_in = 3.5 # approximate isophotal extent of inner semimajor axis\n",

"r_in = 6 # approximate isophotal extent of inner semimajor axis\n",

mwcraig · 2019-08-02T14:49:32Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+   "outputs": [],
+   "source": [
+    "r_in = 3.5  # approximate isophotal extent of inner semimajor axis\n",
+    "r_out = 5.  # approximate isophotal extent of inner semimajor axis\n",


Suggested change

"r_out = 5. # approximate isophotal extent of inner semimajor axis\n",

"r_out = 10. # approximate isophotal extent of inner semimajor axis\n",

mwcraig · 2019-08-02T14:51:10Z

@laurenmarietta -- I've responded to the ones where I have an answer, which is not all of them 😬.

From my perspective, the remaining ones are not critical and I am 💯 on merging once the couple changes I just suggested are accepted.

eteq · 2019-08-16T17:58:45Z

Note the CI failed on the most recent one for a different notebook. The odd thing is that that seemed to work in the most recent master. I've tried restarting to see if that does the job, but if it fails again this might benefit from a rebase to get the CI to pass.

Onoddil

Hi @laurenmarietta I've done a once over review of the first ~2.75 notebooks for final checks. The first two notebooks are basically fine, a few minor grammar things here and there but overall they're really good notebooks. I had some issues with latex formatting, but I think that's a usetex thing (my python falls over on exposed & and _ symbols). There's also a few deprecation warnings here and there, but whether those are deemed acceptable or if they need tidying up is left to you.

The aperture photometry notebook is also good for the most part; I've found a minor bug (or just silly convention issue) in the elliptical aperture part; and I can't run a few cells because I couldn't find the header variable in my notebook, but let me know if that's just me not loading a cell (I ctrl-f'd for it but can't see it...). I will finish looking at the aperture photometry notebook, and have a look at the PSF photometry notebook, hopefully next week.

Onoddil · 2019-09-13T18:08:19Z

notebooks/photutils/01_background_estimation/01_background_estimation.ipynb

+    "##### What is background estimation?\n",
+    "In order to most accurately do photometric analysis of celestial sources in image data, it is important to estimate and subtract the image background. Any astronomical image will have background noise, due to both detector effects and background emission from the night sky. This noise can be modeled as uniform, or as varying with position on the detector. \n",
+    "\n",
+    "The `photutils` package provides tools for estimating 2-dimensional background noise, which can then be subtracted from an image to ensure the most accurate photometry possible.\n",


Minor nitpick, but it might be good to replace "background noise" as a term. Background subtraction involves removing flux from stuff we don't care about (dark current, diffuse sky), but noise could be interpreted as meaning "error" (like data can be noisy even when the source you care about is the only source contributing). Could probably just call it "background flux" instead. Pedantic I know; if no one else agrees then that's fine.

Onoddil · 2019-09-13T18:50:14Z

notebooks/photutils/01_background_estimation/01_background_estimation.ipynb

+   "source": [
+    "#### Data representation\n",
+    "\n",
+    "Throughout this notebook, we are going to store our images in Python using a `CCDData` object (see [Astropy documentation](http://docs.astropy.org/en/stable/nddata/index.html#ccddata-class-for-images)), which contains a `numpy` array in addition to metadata such as uncertainty, masks, or units. In this case, our data is in electrons (counts) per second."


Another super minor quibble, but (and I'm not 100% sure on the grammar here) should this be our data are in electrons? Data being plural and all that...

Onoddil · 2019-09-13T18:55:54Z

notebooks/photutils/01_background_estimation/01_background_estimation.ipynb

+    "cbar.ax.set_yticklabels(labels)\n",
+    "\n",
+    "# Define labels\n",
+    "cbar.set_label(r'Flux Count Rate ({})'.format(xdf_image.unit.to_string('latex')), \n",


I'm guessing this isn't particularly worth investing in, but the xdf_image.unit.to_string('latex') produces a "proper" division ( $\mathrm{\frac{e^{-}}{s}}$ ), rather than something more like e^{-} s^{-1}$. Is there a way to change that for the figures? It just looks a bit less neat to me; again, totally irrelevant and not related to the actual notebook, but would be nice if it were a thing we could change.

Onoddil · 2019-09-13T18:58:43Z

notebooks/photutils/01_background_estimation/01_background_estimation.ipynb

+   "source": [
+    "On the left we have plotted this mask, which has a value of 1 (or True) shown in black where the data is bad, and 0 (or False) shown in white where the data is good. \n",
+    "\n",
+    "After the mask is applied to the data - on the right above - the data values \"behind\" the masked values are shown in white."


Really scraping the bottom of the barrel of things to complain about, does notebook markdown do proper handling of m- and n-dashes? I think these are n-dashes rather than hyphens (which would likely be formatted with –). Can you tell I don't have any real problems with this notebook yet?

Onoddil · 2019-09-13T19:02:13Z

notebooks/photutils/01_background_estimation/01_background_estimation.ipynb

+    "\n",
+    "### Calculate scalar background value\n",
+    "\n",
+    "Here we will calculate the mean, median, and mode using sigma clipping. With sigma clipping, the data is iteratively clipped to exclude data points outside of a certain sigma (standard deviation), thus removing some of the noise from the data before determining statistical values."


Should this have another word or two after mode? Like ...median, and mode of the dataset using sigma clipping? Just to make it clear what we're calculating them on

Onoddil · 2019-09-13T20:25:56Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+    "\n",
+    "At the moment, the positions of our apertures are in pixels, relative to our data array. However, if you need aperture positions in terms of celestial coordinates, `photutils` also includes aperture objects that can be integrated with Astropy's `SkyCoords`.\n",
+    "\n",
+    "Fortunately this is extremely easy when we use the [World Coordinate System (WCS)](http://docs.astropy.org/en/stable/wcs/) to decipher a WCS object from the header of the FITS file containing our image, and then the `to_sky()` method to transform our `EllipticalAperture` objects into `SkyEllipticalAperture` objects. \n",


Should this be and then use the to_sky() method?

Onoddil · 2019-09-13T20:28:15Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+    "    theta = obj.orientation.value  * u.rad\n",
+    "    \n",
+    "    # Convert the theta from radians from X axis to the radians from North \n",
+    "    x_to_north_angle = (90. + header['ORIENTAT']) * u.deg\n",


I get NameError: name 'header' is not defined, but cannot see anywhere in the notebook that header is previously called. Should there be a cell somewhere which has the same with file as f: header = blah.header bit as I recall from a previous notebook?

Onoddil · 2019-09-13T20:29:15Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+   "source": [
+    "Now that we have aperture objects that fit our data reasonably well, we can finally perform photometry with the `aperture_photometry` function. This function takes the following arguments:\n",
+    "\n",
+    "* **`data`** - the background-subtracted data array on which to perform photometry.\n",


If you changed the other time I mentioned changing these hyphens to n-dashes these should change as well; otherwise if you ignored me the first time feel free to ignore me the second time!

Onoddil · 2019-09-13T20:30:48Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "The `aperture_sum` value is what reports the number of electron counts within the aperture: 3.47 e<sup>–</sup>/s.\n",


The changes in the definition of theta above mean that you don't get this value; I get 0.9 e/s or so for this number now. I can only assume once the bug is fixed this'll go back to normal, but this is a good litmus test... Or the number needs fudging!

After fixing the degrees to radians conversion above I get the value quoted in the text here, so please disregard my above comment on this line.

Onoddil · 2019-09-13T20:31:46Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+   "outputs": [],
+   "source": [
+    "# The CCDData mask will be automatically applied\n",
+    "sky_phot_datum = aperture_photometry(xdf_image, sky_elliptical_apertures[0], wcs=wcs)\n",


Not being able to run the cell above with the unknown header variable means you can't run this cell either, but that presumably will go away once the other issue is fixed (or you please tell me where I went wrong with loading the header in the first place)

Onoddil

HI @laurenmarietta, just to tag onto my review last week, here is (almost all of) the rest of the aperture photometry notebook review. I revised some of the comments I made in my last re: my poor attempt to debug the ellipse misalignment plotting issue. I didn't quite run the notebook to the end (although I can only assume the final plot, basically the same as one I did run, is fine), as I ran into issues with the background subtracted photometry cell where presumably something has changed in the internal photutils backend which I will look into more and hopefully have a suggestion as to what needs updating soon.

Onoddil · 2019-09-16T15:18:57Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+    "\n",
+    "Since our elliptical apertures are distinct apertures at distinct positions, we need to do a little more work to get a single table of photometric values.\n",
+    "\n",
+    "(This step will take a while, almost 5 minutes, so hang tight!)"


Potentially silly idea, and not related to this notebook, but would this wrapper as a convenience function actually be useful as a function available in photutils? This notebook could then be slightly streamlined to avoid having to make the function, and could just call the "hidden" convenience wrapper; but then the code would be more readily available to users who don't then have to copy-paste this cell into their code to get the functionality we're telling them is useful for aperture photometry. Just a thought; I'm happy to do the PR if people think this might be something worth adding (and updating the notebook with accordingly).

Onoddil · 2019-09-16T15:37:59Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+    "    position = (obj.xcentroid.value, obj.ycentroid.value)\n",
+    "    a = obj.semimajor_axis_sigma.value * r\n",
+    "    b = obj.semiminor_axis_sigma.value * r\n",
+    "    theta = obj.orientation.value\n",


Turns out I can't read the documentation and the issue isn't in the angle definitions but that obj.orientation is a Quantity with value in degrees by default, so this line needs to be theta = obj.orientation.to(u.rad).value to convert to the radians units needed for plotting. See here for an up-to-date version of an example combining these two codes, with hopefully only the one change needed.

Onoddil · 2019-09-16T15:42:08Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "The `aperture_sum` value is what reports the number of electron counts within the aperture: 3.47 e<sup>–</sup>/s.\n",


After fixing the degrees to radians conversion above I get the value quoted in the text here, so please disregard my above comment on this line.

Onoddil · 2019-09-16T15:46:10Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+    "\n",
+    "<h3>Exercise:</h3><br>\n",
+    "\n",
+    "Re-calculate the photometry for these elliptical apertures - or just a subset of them - using the <code>subpixel</code> aperture placement method instead of the default <code>exact</code> method. How does this affect the count sum calculated for those apertures?\n",


- -> – instance around the subset of them subclase.

Onoddil · 2019-09-16T15:54:22Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+    "plt.xscale('log')\n",
+    "plt.title('Count Rate v. Aperture Area')\n",
+    "plt.xlabel('Aperture Area [pixels$^2$]')\n",
+    "plt.ylabel(r'Flux Count Rate ({})'.format(xdf_image.unit.to_string('latex')))"


Just another comment to say that the to_string('latex') formatting isn't particularly neat (being \frac{e-}{s} instead of a nicer, single-level e- s^{-1}. Again as with previously, not sure if there's anything we can do about it but would be nice if it were possible to change it; is there a reason we're not just hard-coding the units? Are they likely to change?

Onoddil · 2019-09-16T16:06:19Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+    "\n",
+    "Now that our apertures have been defined, we can do photometry with them to estimate and account for the background. The aperture correction is calculated by:\n",
+    "- Calculating the count rate within each annulus using `aperture_photometry`\n",
+    "- Dividing each annulus' count rate by each annulus' area to get the mean background value for each annulus\n",


Might be good to swap these around each annulus' count rate -> the count rate of each annulus, each annulus' area -> the area of each annulus, to avoid the slightly awkward floating possessive quote marks.

Onoddil · 2019-09-16T16:08:01Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+    "- Calculating the count rate within each annulus using `aperture_photometry`\n",
+    "- Dividing each annulus' count rate by each annulus' area to get the mean background value for each annulus\n",
+    "- Taking the mean of those annulus means to get a mean background value for the entire image\n",
+    "- Multiplying the global background mean value times the area of each elliptical photometric aperture, to get the estimated background count rate within each aperture\n",


value times the area -> value by the area, to make the sentence read more "Multiplying A by B", instead of "Multiplying A times B", which is slightly redundant. Or perhaps it could be "Calculating A times B", which would be Multiplying -> Calculating.

Onoddil · 2019-09-16T16:10:02Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "You might have noticed that these background count rates are *really* small. In this case, this is to be expected - since our example XDF data is a high-level science product (HLSP) that already has already been background-subtracted."


One more - -> – replacement :)

Onoddil · 2019-09-16T16:11:30Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+   "source": [
+    "---\n",
+    "## About this Notebook\n",
+    "**Authors:** Lauren Chambers (lchambers@stsci.edu), Erik Tollerud (etollerud@stsci.edu), Tom Wilson (towilson@stsci.edu) Clare Shanahan (cshanahan@stsci.edu)\n",


Missing a comma between me and Clare in the list here

Onoddil · 2019-09-16T16:16:05Z

notebooks/photutils/03_aperture_photometry/03_aperture_photometry.ipynb

+   "outputs": [],
+   "source": [
+    "# Calculate the mean background level (per pixel) in the annuli \n",
+    "bkg_area = [annulus.area() for annulus in elliptical_annuli]\n",


This line gives me an error TypeError: 'float' object is not callable. I will try to dig into why it's doing that if I get time later, but for now that's as far as I can get in the cells.

All that needed changing here was annulus.area() -> annulus.area, where it appears area changed from a function to a property.

eteq · 2019-11-27T21:57:26Z

One more note on this PR: probably we want to just remove the PSF one for now since it's a scaffold. If someone decides they want to take it on they certainly can, but in the meantime a partially-done notebook doesn't really belong in this repo.

eteq · 2019-12-10T23:42:57Z

After some discussion at the 2019 Astropy Coordination Meeting, @mwcraig has volunteered to help finish out this notebook (as it dove-tails well with some of his other work building a ccd reduction guide). He will take this branch and finish it out in a separate PR.

cc @mwcraig @adrn @kelle @laurenmarietta @nmearl

Lauren Chambers added 2 commits May 20, 2019 15:28

Initial commit of photutils notebooks

e684af4

Rename notebooks, and implement previous edits across all notebooks

907b637

laurenmarietta requested a review from astro-josh as a code owner May 20, 2019 20:46

This was referenced May 20, 2019

Photutils tutorial notebook #2: source detection eteq/csi-stsci-notebooks#3

Closed

Photutils tutorial notebook #3: aperture photometry eteq/csi-stsci-notebooks#4

Closed

Lauren Chambers added 3 commits May 20, 2019 16:58

Fixes so HTML renders correctly

ef462b5

Add .gitignore from spacetelescope#99 and add apertures PNG

d0a8698

Maybe adding a tags will help...?

fe62145

attempting to fix ci errors with latex

4f0b732

astro-josh reviewed May 22, 2019

View reviewed changes

mwcraig reviewed Jun 3, 2019

View reviewed changes

Merge branch 'master' of github.com:spacetelescope/notebooks into pho…

4fe94df

…tutils # Conflicts: # notebooks/.gitignore

laurenmarietta requested a review from eteq as a code owner July 30, 2019 18:01

Lauren Chambers added 2 commits July 31, 2019 10:17

First round of edits

dc17e7c

More fixes

852a3f2

laurenmarietta force-pushed the photutils branch from 6cf2415 to 852a3f2 Compare July 31, 2019 14:23

More edits from @mwcraig

b19ad49

mwcraig reviewed Aug 2, 2019

View reviewed changes

Onoddil reviewed Sep 13, 2019

View reviewed changes

Onoddil reviewed Sep 16, 2019

View reviewed changes

mwcraig mentioned this pull request Jan 5, 2020

Photutils tutorial notebooks (replacement for #101) #124

Open

	"To estimate the local background for each aperture, measure the counts within annulus apertures around (but not including!) each source. In our example, we defined elliptical apertures with `r = 3` pixels to measure the counts within each source. To calculate the background for each source, let's measure the counts elliptical annuli between `r = 3.5` pixels and `r = 5` pixels."
	"To estimate the local background for each aperture, measure the counts within annulus apertures around (but not including!) each source. In our example, we defined elliptical apertures with `r = 3` pixels to measure the counts within each source. To calculate the background for each source, let's measure the counts elliptical annuli between `r = 6` pixels and `r = 10` pixels."

	"r_in = 3.5 # approximate isophotal extent of inner semimajor axis\n",
	"r_in = 6 # approximate isophotal extent of inner semimajor axis\n",

	"r_out = 5. # approximate isophotal extent of inner semimajor axis\n",
	"r_out = 10. # approximate isophotal extent of inner semimajor axis\n",

[WIP] photutils tutorial notebooks #101

Are you sure you want to change the base?

[WIP] photutils tutorial notebooks #101

Conversation

laurenmarietta commented May 20, 2019

astro-josh commented May 21, 2019

laurenmarietta commented May 21, 2019

astro-josh commented May 21, 2019

astro-josh left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwcraig left a comment

Choose a reason for hiding this comment

laurenmarietta commented Jul 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwcraig commented Aug 2, 2019

eteq commented Aug 16, 2019

Onoddil 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

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

Onoddil left a comment • edited Loading

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

eteq commented Nov 27, 2019

eteq commented Dec 10, 2019

astro-josh left a comment •

edited

Loading

Onoddil left a comment •

edited

Loading