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

BUG: dendrogram's labels not displaying correctly after updating to 0.14 #3822

Closed
alessioU opened this issue Jul 23, 2014 · 11 comments
Closed
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.cluster

Comments

@alessioU
Copy link

In Scipy 0.13 in scipy.cluster.hierarchy in the function _plot_dendrogram, to set the proprieties of the labels like rotation and font size, the function matplotlib.pylab.setp is used and the labels are displayed correctly.
Instead in Scipy 0.14, to set the proprieties of the labels like rotation and font size, it's used a map that simply calls for example just set_rotation or set_size but the labels are not rotated and not resized as you can see in the screenshot. I think matplotlib.pylab.setp does more than just calling set_rotation or set_size.
screenshot - 07232014 - 09 11 44 pm

@argriffing
Copy link
Contributor

Could be related to #3790?

@alessioU
Copy link
Author

Well, my guess is that @gregcaporaso was experiencing this same issue because he was using Scipy 0.14, but if he had tried to use Scipy 0.13 he would have found that scaling and rotating, when there are a lot of labels, was already implemented and so @ebolyen wouldn't had to rewrite code that was already there and working in the previous version of Scipy.

I think that the issue can be resolved by replacing the map calls with matplotlib.pylab.setp as it was in 0.13.

Edit: Actually in 0.13 if you use orientation='right' the code is different than when you use orientation='top'(default) and for some reason the labels are not resized automatically but this feature can be added easily.

@ebolyen
Copy link

ebolyen commented Jul 24, 2014

This issue certainly prompted #3790. However our specific need was a larger dendrogram for @gregcaporaso's presentation.

@gregcaporaso
Copy link
Contributor

Thanks @alessioU. In addition to scaling and rotating the font size, it can also be useful to be able to create bigger images. That's something we'd benefit from for displaying (small to medium-sized) phylogenetic trees as dendrograms in scikit-bio. Will that be possible when this bug is fixed? If not, and if that functionality is something that you'd be interested in, @ebolyen and I could coordinate to submit a pull request. We're happy to do it, but just don't want to do the work if there is not interest in adding this functionality.

@rgommers rgommers added the defect A clear bug or issue that prevents SciPy from being installed or used as expected label Nov 27, 2014
@WarrenWeckesser
Copy link
Member

This bug is still in master. @jamestwebber, if you are interested in continuing your contribution to scipy, could you take a look at this? Did arguments such as leaf_rotation work for you after you added the ax argument to dendrogram?

@jamestwebber
Copy link
Contributor

Sorry that I broke this, I'll take a look at this later today!

jamestwebber added a commit to jamestwebber/scipy that referenced this issue May 28, 2015
@jamestwebber
Copy link
Contributor

A couple notes while I'm working on this (I think I have it fixed at this point):

The reason we don't want to simply revert to the old version is because it used pylab. pylab is really designed for interactive work and not recommended for use in programs. I changed the code to only use pylab when needed, because I wanted to be able to generate dendrograms a) without starting an interactive instance and b) on an arbitrary axes instance that I supply, rather than the active one.

However there is no "setp" in the regular matplotlib API, so I tried to emulate the existing code as best I could. I didn't really analyze whether it was doing the right thing--in retrospect it's got some weird behavior.

As @alessioU noted, the four different orientations all do different things. "top" autoscales and rotates based on the number of labels. "bottom" is supposed to autoscale based on p, a "truncate mode" parameter. "left" and "right" don't autoscale or rotate the labels without being told to. That's how the code was when I started, so I left it that way, but it seems reasonable to fix that while I'm in there. The only question is: should I use the number of labels, or p? I chose the former for now but that's a simple decision to make before merging.

@jamestwebber
Copy link
Contributor

I guess another (in hindsight obvious) question is what the left/right orientations should do with their label rotation. The current auto-rotate stuff is geared toward the top/bottom orientations--it has horizontal labels unless you have a ton of leaves and then it rotates to vertical (first going to 45 degrees).

Seems like I should probably just keep the left/right orientations with horizontal labels--there's no downside as far as I can tell. If the user specifies a rotation then that wins, otherwise it's horizontal.

@WarrenWeckesser
Copy link
Member

I guess another (in hindsight obvious) question is what the left/right orientations should do with their label rotation.

The conservative answer to questions of the form "What should it do?" is "Whatever it did in scipy 0.13." That way we preserve backwards compatibility.

For that, though, you'll need to build scipy 0.13. I tried building 0.13.3, but I got a cython error. That's as far down the debugging rabbit hole that I had time to crawl. Perhaps someone else can take a look.

@jamestwebber
Copy link
Contributor

Well the answer was "nothing, unless the user specified", which is what I ended up doing. Which makes sense--y-axis labels don't need to be rotated for readability, so unless the users really wants that to happen they can stay the way they are.

rgommers pushed a commit to rgommers/scipy that referenced this issue Aug 16, 2015
…ygh-3822.

Also, I added the resizing code to the other orientations. They're currently
all based on the number of labels but that could be changed to the "p"
parameter easily. This way seems simpler though.

Removed the autorotate on left/right orientations.

Explicit checks for None to allow value of 0.
@rgommers
Copy link
Member

Multiple new fixes in addition to the ones from @jamestwebber just went into gh-5164. Review of that PR would be much appreciated.

Also, some larger dendrograms for testing would be very welcome. Despite the open issues and the many stackoverflow posts about dendrogram, it's very difficult to find examples that (should) work.

rgommers pushed a commit to rgommers/scipy that referenced this issue Oct 13, 2015
…ygh-3822.

Also, I added the resizing code to the other orientations. They're currently
all based on the number of labels but that could be changed to the "p"
parameter easily. This way seems simpler though.

Removed the autorotate on left/right orientations.

Explicit checks for None to allow value of 0.

(cherry picked from commit d7fe77a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.cluster
Projects
None yet
Development

No branches or pull requests

7 participants