bradenmacdonald / astrodendro

Deprecated. Python package for computation, visualization, and quantified analysis of astronomical dendrograms
2 stars 0 forks source link

Astrodendro mass-radius plots #5

Open keflavich opened 11 years ago

keflavich commented 11 years ago

For making the style of plots Jens Kauffmann likes, e.g. Kauffmann 2013 comparing Orion with the Brick. Same as previous pull request onto dendro-core, but this time with the right target.

https://github.com/bradenmacdonald/dendro-core/pull/1#issuecomment-12450895

Thanks for clarifying the split @bradenmacdonald. Might be worth referencing that comment - or copying it - in the docs.

bradenmacdonald commented 11 years ago

Sorry for taking so long to review this. I tested it and it does work, but I think there are a couple changes we should make to the code before merging it.

  1. Each Leaf/Branch has an .items property that contains its immediate children, and using that will be much more efficient than using [d for d in item.descendants if d.level==item.level++]
  2. Since each python variable for a Leaf or a Branch is just a pointer (behind the scenes), I think it's actually less efficient to have these separate methods that return only specific properties (npix, f_sum), rather than methods that just return a list of matching branches. Of course, the only way to know for sure is to benchmark it, but I feel that the increased code clarity that comes with the latter approach makes it the best way to go.

Here's an example. I think this is working (please test) for the 'brightest' plot mode; I haven't implemented the other version.

def _build_lines(self, items, brightest=True):
        for item in items:

            color = self._color_lambda(item)

            if brightest:
                ### Changes start here ########
                brightest_children = []
                def add_brightest_children(current_item):
                    brightest_children.append(current_item)
                    if type(current_item)==Branch:
                        brightest_child = max(current_item.items, key=lambda i: i.f_sum)
                        add_brightest_children(brightest_child)
                add_brightest_children(item)
                xvalues, yvalues = zip(*[ (i.npix, i.f_sum) for i in brightest_children ])
            else:
                # TODO: normal (non-brightest) mode
            self._plot_values[item.idx] = { 'color': color, 'values': [[xvalues,yvalues],] }

one could also implement this as follows:

            xvalues, yvalues = [],[]
            if brightest:
                def add_brightest_children(current_item):
                    xvalues.append(current_item.npix)
                    yvalues.append(current_item.f_sum)
                    if type(current_item)==Branch:
                        brightest_child = max(current_item.items, key=lambda i: i.f_sum)
                        add_brightest_children(brightest_child)
                add_brightest_children(item)
            else: ...