Closed astrofrog closed 11 years ago
ChrisBeaumont
commented
11 years ago This looks nice. I'd add subsection headings to label and distinguish the discussions of min_value and delta. Also, I'd consider using the terms "items" or "pixels" in some of the places you use the term "values" (in my head, a pixel/item has a value, but is not itself merely a value).
astrofrog
commented
11 years ago @ChrisBeaumont - I've addressed your comments. Could you review this once more? I also included smaller images (rather than rescaling large images) and increased the font size a little. I got rid of the original commit to avoid large binary diffs.
Do you think this looks ok to merge? Let me know if you have other comments/suggestions!
ChrisBeaumont
commented
11 years ago Looks good, apart from two typos (data is vs data are, in future vs in the future)
On Wednesday, July 31, 2013, Thomas Robitaille wrote:
@ChrisBeaumont https://github.com/ChrisBeaumont - I've addressed your comments. Could you review this once more? I also included smaller images (rather than rescaling large images) and increased the font size a little. I got rid of the original commit to avoid large binary diffs.
Do you think this looks ok to merge? Let me know if you have other comments/suggestions!
— Reply to this email directly or view it on GitHubhttps://github.com/dendrograms/dendro-core/pull/65#issuecomment-21851362 .
astrofrog
commented
11 years ago Thanks! I'll merge if the tests pass.
@ChrisBeaumont - this is just a draft for now, and I plan to add correctly-sized and named versions of the plots (and rebase to avoid large binary diffs), but I'd be interested in feedback on what's already there. Once we finalize this page, then I can go back to making the screencast and then embed it at the top of the page for anyone who can watch it instead of reading the text.
Preview of the docs: https://dendrograms-astrofrog-v2.readthedocs.org/en/latest/algorithm.html
What do you think? Too many plots? Not clear enough?