Added API docs, improved docstrings

[astrofrog]

This adds API docs for the public API, and means that we can hyperlink class and methods names in the docs to the docstrings.

This helped me review the API too, so I have a few questions:

• we sometimes refer to structures, and sometimes nodes - to avoid confusion, should we always use structure/structures? (e.g. prefix_structures or all_structures).
• in the Structure class, we sometimes use two properties such as indices_all and indices to refer to including and excluding the sub-tree, and sometimes use one method with a subtree option. I feel like this should be more consistent - what do you think?

I will continue to work on this and will add an API docs page for the PP/PPV catalog makers so we can link to those too.

cc @ChrisBeaumont

[ChrisBeaumont]

+1 for moving from nodes->structures.

I'm also fine with changing all_indices to indices(subtree=True). I think this is better than going the other direction, to minimize the number of functions.

Come to think of it, the cataloging routines are currently using indices and values, but they should be using indices_all and values_all (or whatever they are renamed to). We should change that.

[astrofrog]

Ok, I think I'll stop here for now otherwise it's going to be hard to review ;)

Basically, this adds human-readable API pages and updates the docs to point to them whenever appropriate. I also then updated docstrings to make them more readable in the docs, fix syntax issues, etc.

I'll make the changes you agree with. Along those lines, should we then use peak() instead of get_peak(), or should we use get_indices instead of indices, or is it ok to have the two be different?

[astrofrog]

Ok, I've now changed values and indices to methods. We could actually just leave it as is, but if we were to rename something, I'd be in favor of changing get_npix to npix (the other get_ methods are more obviously derived objects rather than properties of the structure).

@ChrisBeaumont - what do you think? Apart from that, good to merge?

[astrofrog]

Oops, I'll need to fix the failed tests first before we merge.

[astrofrog]

Turns out 2to3 changes values() to wrap it with a list (see last commit) - seems easy enough to fix here, but worth thinking about whether that's ok.

[astrofrog]

@ChrisBeaumont - I addressed your comment above about the ScalarStatistic API, though it does lead to some ugly code in some cases:

    self.values = lambda subtree=True: values.astype(np.float)
    self.indices = lambda subtree=True:indices

but maybe there's a way to improve this? If you want, we could merge this and you can see if you can find a way to tidy up the use of the functions in ScalarStatistic? (if Travis passes, that is)

Note I've now changed subtree to default to True everywhere and in our code, I've made it so we always explicitly specify it and never rely on the default (except in one tests which checks the default).

[astrofrog]

(btw, I know you are at SciPy, so no rush, just reply whenever you have a free minute)

[astrofrog]

(Hope the talk is going well!! :) )

[astrofrog]

A preview of the docs in this PR (with public API docs) is available here: https://dendrograms-astrofrog-v2.readthedocs.org/en/latest/

[astrofrog]

Just for info, I'm working on cleaning up the API a little so as not to need the lambda functions.

[astrofrog]

@ChrisBeaumont - I've cleaned things up a little so as not to require lambda functions (which I'd introduced in a previous commit). I tried making ScalarStatistic have the same API (for values and indices) as Structure, but that turned out to be a mistake because it leads to hacky code with lambda functions, and I don't think it makes sense to treat indices and values as dynamic values once passed to the ScalarStatistic class, so they should just be properties there.

The solution I came up is simply to require _make_catalog to take a list of structures, not ScalarStatistic. However, if you want _make_catalog to be able to take a list of ScalarStatistic then the correct solution in my opinion is to have:

if isinstance(struct, ScalarStatistic):
    stat = struct
else:
    stat = ScalarStatistic(struct.values(subtree=False), struct.indices(subtree=False))

in _make_catalog. Note that this is different from the if statement I had originally. I can add this in if you agree. I think _make_catalog will need to be re-thought anyway once we start using vector statistics (that's for a future pull request!).

Anyway, I think the tests should all pass now, and this is ready for final review.

[astrofrog]

Travis is passing, so let me know once you're happy for this to be merged :)

[ChrisBeaumont]

looks good!

[astrofrog]

Thanks for reviewing! And I promise that now I'll keep pull requests more bite-size ;)

[ChrisBeaumont]

Don't let anyone tell you you don't know how to refactor like a champ :)

At this point, I should be able to build a dendroram+catalog on real data with both the IDL code and the python code. I'll see if they match up.

[astrofrog]

Good idea! :)