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] Fix an error when using latest pyqtgraph develop snapshot #1662

Merged
merged 2 commits into from
Oct 14, 2016

Conversation

ales-erjavec
Copy link
Contributor

  • Fix an AttributeError when using latest pyqtgraph develop branch.
  • Lock aspect ratio on the plot view.

@codecov-io
Copy link

codecov-io commented Oct 14, 2016

Current coverage is 88.73% (diff: 100%)

Merging #1662 into master will not change coverage

@@             master      #1662   diff @@
==========================================
  Files            78         78          
  Lines          8150       8150          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           7232       7232          
  Misses          918        918          
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 95cea08...461b846

@lanzagar
Copy link
Contributor

Looks OK.
I just have a question about locked aspect ratio - do we want it to behave differently here than elsewhere (e.g. scatterplot)? It was probably done to avoid issues/complications with axis labels and their angles?
I don't mind it, just think that it would be better to have it behave consistently across similar visualizations.

@ales-erjavec
Copy link
Contributor Author

Yes labels and text would get misaligned, so locking the aspect fixes it. However biplots should/need to preserve aspect ratio (i.e. preserve angles).

Note that MDS also uses locked aspect ratio (to preserve distances). So not all visualizations are consistent in this regard.

Come to think of it, Correspondence Analysis might also need this (it is a form of a biplot).

@lanzagar
Copy link
Contributor

I agree that it should be locked here, let's do it in others as well (where needed).
I guess we can leave scatter plot unlocked, where it is not needed, and might even be desirable. The different behaviour should not confuse users too much (and it is justified).

@lanzagar lanzagar merged commit 07dafdf into biolab:master Oct 14, 2016
@astaric astaric modified the milestone: 3.3.9 Nov 28, 2016
@ales-erjavec ales-erjavec deleted the pyqtgraph-develop branch January 27, 2017 10:40
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.

4 participants