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

Allow forcing a fix axis scale ratio to maintain arrow lengths #1197

Merged
merged 10 commits into from
Sep 26, 2018

Conversation

dabana
Copy link
Contributor

@dabana dabana commented Sep 24, 2018

Addresses issue #1187.

A new scaleratio parameter is added to the create_quiver function to allow forcing a fix scale ratio to the axis of the plot through the scaleratio option in the Layout. A distortion is introduced to the arrows length, angle and shape by having a scale ratio different than 1. In this pull request, this distortion is reversed by multiplying or dividing by scaleratio at key steps of the arrows construction.

@jonmmease
Copy link
Contributor

Hi @dabana, I haven't reviewed/tested this in detail yet but at a high level this looks great! In the meantime, could you add a simple test to plotly/tests/test_optional/test_figure_factory/test_figure_factory.py? Just follow the pattern of the existing tests to make sure that the figure that's produced when scaleratio is set has the structure you expect.

Thanks for the great contribution!

@dabana
Copy link
Contributor Author

dabana commented Sep 25, 2018

It will be done.

@dabana
Copy link
Contributor Author

dabana commented Sep 26, 2018

There were no test class for the quiver plot, so I created it. But when trying out the tests with Nose:

nosetests -w plotly/tests/test_optional/test_figure_factory

I get the following import error:

ModuleNotFoundError: No module named 'retrying'

even though the retrying module is installed in my environment.

@jonmmease
Copy link
Contributor

Thanks @dabana, this looks great! Thanks for the test, docstring, and example. I tried out several other examples and this makes the interpretation of the arrow lengths much more accurate.

Also, let me know if there's anything else you're interested in doing to improve create_quiver, it's so helpful to have contributors who also use the figure factory in their work 🙂

@jonmmease jonmmease merged commit e0da828 into plotly:master Sep 26, 2018
@jonmmease jonmmease added this to the v3.3.0 milestone Sep 26, 2018
@dabana
Copy link
Contributor Author

dabana commented Sep 26, 2018

Great! This PR is a good first step toward issue #1187. I would like to make the choice of scaleratio a little more automatic for the user.

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.

2 participants