Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Photutils getting started tutorial #99

Closed

Conversation

bsipocz
Copy link
Member

@bsipocz bsipocz commented Sep 23, 2015

This is to partially address #88.

cc: @larrybradley, @cdeil

@cdeil
Copy link
Member

cdeil commented Sep 23, 2015

@bsipocz – Thanks!

Maybe change the title from "Getting Started" to "Getting started with photutils" ... IMO the work "photutils" or "photometry" should be in the title, maybe "source detection" also.

Also having sub-titles that include the key words like "source detection" and "photometry" would be nice, so that it's more prominent and can be linked to individually.

And can you please link to the photutils project in the first sentence of the notebook and again at the end something like "If you want to learn more, go to the photutils docs [add link]".

The line "from astropy.stats import median_absolute_deviation as mad" can be removed I think, it's not used.

Otherwise, 👍 !

@bsipocz
Copy link
Member Author

bsipocz commented Sep 23, 2015

@cdeil - Good points, thanks. I'll update this tomorrow.

@larrybradley
Copy link
Member

Thanks, @bsipocz. For input cell 1, you can remove the import of median_absolute_deviation (only mad_std is used). I've updated this in the photutils "Getting Started" docs.

@bsipocz
Copy link
Member Author

bsipocz commented Sep 24, 2015

Notebook is updated.

@eteq
Copy link
Member

eteq commented Oct 13, 2015

@bsipocz - looks like a good start. Were you thinking of this as a starting point and are planning to add more, or want this as-is?

For the content that's actually there, can I suggest a bit more introductory material? I.e., say briefly what the image is, and explain just a bit more about what's actually going on in each processing step. Personally I think it's better to view these tutorials as though you're teaching a class, as opposed to writing documentation. Does that make sense?

A few other "mechanical" bits: you'll need to add a metadata section to the notebook itself, as well as a requirements.json file. (See https://github.com/astropy/astropy-tutorials/blob/master/CONTRIBUTING.md#procedure for more on this.) You'll also want to remove all the output from the notebook before this can be merged - it's ok to do it for now so we can see how it looks, but the final tutorial to be merged should have no outputs (and ideally, no outputs in the commit log.)

@eteq
Copy link
Member

eteq commented Oct 13, 2015

Oh, and along the lines above, you'll note that all the tutorials have "exercises" - I'd suggest you add just one or two to this so that they give the reader an opportunity to engage in the tutorial.

@bsipocz
Copy link
Member Author

bsipocz commented Oct 14, 2015

@eteq - Thanks for the feedback, it's most useful. I (we) intended to have this in as is, and then maybe add another one (or extend this) later when someone has more time to spare.
I hope to be able to get back to it in the weekend, and add a bit more text and an exercise.

@eteq
Copy link
Member

eteq commented Nov 8, 2016

@bsipocz - I happen to notice this well looking at something else. I imagine it just slipped our minds... did you still want to take a crack at getting this in?

@bsipocz
Copy link
Member Author

bsipocz commented Nov 8, 2016

I won't have time to finish this off now but can defer it until later for a hackday. With the recent changes most of this needs to be replaced probably.
So it's up to you, either close it, or leave it as a reminder that we need to add a basic photutils tutorial at some point.

@adrn
Copy link
Member

adrn commented May 11, 2018

Hey @bsipocz! I'm going to close this for now in case any new contributors would like to take over the photometry tutorial (I don't want them to be discouraged by the open PR). Feel free to reopen if you have time to finish this off!

@adrn adrn closed this May 11, 2018
@bsipocz
Copy link
Member Author

bsipocz commented May 11, 2018

Unless we do a tutorial focused sprint day it's super unlikely that I'll have any time to pick this up any time soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants