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

Add tutorial/docs for visualization #33

Merged
merged 9 commits into from
Jun 25, 2013

Conversation

astrofrog
Copy link
Contributor

Once #29 is merged, I will work on this.

…onvenience, and also speed up GUI by a factor of 2x by avoiding calling ``draw()`` twice for every slider action (the slider calls ``draw()`` by default).
@astrofrog astrofrog mentioned this pull request Jun 25, 2013
@astrofrog
Copy link
Contributor Author

(this is still work in progress - I'll post a link to a built version on RTD once it's ready for review)

@astrofrog
Copy link
Contributor Author

@ChrisBeaumont - this is ready for review, but readthedocs is acting up (I know it builds with the correct dependencies because it worked once, but it's refusing to build on demand for my fork) so you'll need to build locally to see the new documentation page. Let me know what you think about the plotting API!

@ChrisBeaumont
Copy link
Contributor

Looks great! This puts the old IDL code to shame :)

One minor point: in stead of d.plotter().plot_tree(ax, ...), what do you think about d.plotter(ax).plot_tree()?

Also, is this compatible with plotting with APLpy?

@astrofrog
Copy link
Contributor Author

Hmm, that's not a bad idea. In that case, the plotter becomes single purpose (i.e. tree or contour). Maybe I should then make two separate Plotter classes since only the tree one needs the cached positions? Then one would avoid recomputing the cached positions when calling plotter(ax) to make the contour plots. What do you think? I'm flexible on this.

@astrofrog
Copy link
Contributor Author

The problem with the latter option is then once has e.g. d.contour_plotter.plot_contour which is a little redundant, so maybe not a good idea...

@ChrisBeaumont
Copy link
Contributor

You're right, that doesn't feel quite right. The way it is is fine

@astrofrog
Copy link
Contributor Author

Regarding APLpy - I'm planning to try and make it possible to use them together, and will add a section here once I get an example working :) (will open an issue for that)

Is this ok to merge?

@ChrisBeaumont
Copy link
Contributor

👍

astrofrog added a commit that referenced this pull request Jun 25, 2013
Add tutorial/docs for visualization
@astrofrog astrofrog merged commit e4bf094 into dendrograms:master Jun 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants