Make structures_dict private

[ChrisBeaumont]

(builds on top of #52. Will rebase)

@astrofrog, I'm not sure if you think this is needed. I felt it made more sense to hide structures_dict from users of the Dendrogram class, and instead give Dendrograms container-like interfaces like len, getitem, __iter__, etc. Some of these functions already exist. This PR makes structures_dict private.

Along the same lines, all_structures, __iter__ and prefix_structures all do very similar things. prefix_structures is needed in situations where the order is important, but it has a less user-friendly name. Do you think the three methods should be consolidated somehow?

[astrofrog]

@ChrisBeaumont - I think that it makes sense to consolidate the methods you mention. Maybe we could at least consolidate all_structures and prefix_structures into a single method get_structures with a keyword argument? Is there a large overhead for prefix_structures? If not, we could even just always return the ordered structures?

[ChrisBeaumont]

The worst-case overhead for prefix_structures is building a second list that stores references to each of the nodes. I think it would be fine to use the prefix order in all_structures -- we just need to document that this order is guaranteed by all_structures, and make sure that doesn't change.

[ChrisBeaumont]

What do you think of this? It uses a single all_structures property, which yields the structures in prefix order.

The main downside here is that if we decide we do need multiple options for how the tree is traversed, we can't add/change any kwargs -- we'd have add new methods/properties, or break the API by changing all_structures into a method.

[astrofrog]

This seems fine to me. Just to check, what is the motivation for making this a property rather than a method, which would then allow kwargs in future?

[ChrisBeaumont]

There's no real reason - do you think a method is better? On Jul 13, 2013 1:24 PM, "Thomas Robitaille" notifications@github.com wrote:

This seems fine to me. Just to check, what is the motivation for making this a property rather than a method, which would then allow kwargs in future?

— Reply to this email directly or view it on GitHubhttps://github.com/dendrograms/dendro-core/pull/53#issuecomment-20923232 .

[astrofrog]

I guess that a method means we don't have to worry if we do want to have the option to get structures sorted differently in future. We could even name the method get_sorted_structures to make it clear some sorting is happening. I don't have strong opinions here, but just want to point out the options. I'll let you decide :)

[astrofrog]

@ChrisBeaumont - how do you think we should proceed with this?

[ChrisBeaumont]

I'm partial to a property here, since it feels more like data access than anything else. I vote for using the interface proposed in this PR. If we really need to distinguish sorted / unsorted structures in the future, we can add a new method. But I think it might not be an issue.

[astrofrog]

Sounds good - in this case, is this ready to merge?

[ChrisBeaumont]

Sounds good to me