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

Fix stacked_violin plot bug #525

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@outlace
Copy link

outlace commented Mar 10, 2019

num_rows was incorrectly calculated when swap_axes=False in the stacked_violin plot. This fixes issue #405

Fix stacked_violin plot bug
`num_rows` was incorrectly calculated when `swap_axes=False`.
@falexwolf

This comment has been minimized.

Copy link
Member

falexwolf commented Mar 10, 2019

Thank you for this! Unfortunately, it breaks the tests. It's such a small change, but I guess @fidelram had something in mind when setting it the way he did...

@outlace

This comment has been minimized.

Copy link
Author

outlace commented Mar 10, 2019

I’ll have another look. But it doesn’t work the way it is now either

@fidelram

This comment has been minimized.

Copy link
Collaborator

fidelram commented Mar 11, 2019

@outlace I will check the problem with the test and integrate your changes in a new PR that addresses #512 and #524 if this is OK with you.

@outlace

This comment has been minimized.

Copy link
Author

outlace commented Mar 11, 2019

@fidelram Sounds good!

@fidelram

This comment has been minimized.

Copy link
Collaborator

fidelram commented Mar 11, 2019

@outlace Curiously, your change causes an error. Without your change I can run the tests correctly without a problem. I remember that I fixed a bug similar to this one that was recently integrated into master (see https://github.com/theislab/scanpy/pull/425/files#diff-b5175ed1415cdbf853646e523cbe8ae0L902). Could it be that you didn't have the latest pull from scanpy and that was causing the error?

@outlace

This comment has been minimized.

Copy link
Author

outlace commented Mar 11, 2019

Possible, I will try pulling the latest master and see if I still get the problem

@fidelram

This comment has been minimized.

Copy link
Collaborator

fidelram commented Mar 20, 2019

@outlace do you think I can close this PR?

@fidelram fidelram closed this Apr 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.