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] FreeViz: 2 issues when no data #2780

Merged
merged 2 commits into from
Nov 27, 2017
Merged

[FIX] FreeViz: 2 issues when no data #2780

merged 2 commits into from
Nov 27, 2017

Conversation

jerneju
Copy link
Contributor

@jerneju jerneju commented Nov 16, 2017

Issue

Widget crashes when there is no data and

  1. Radius slider is moved
  2. Class density, etc. is changed
Description of changes
Includes
  • Code changes
  • Tests
  • Documentation

@codecov-io
Copy link

codecov-io commented Nov 16, 2017

Codecov Report

Merging #2780 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2780      +/-   ##
==========================================
+ Coverage   76.09%   76.09%   +<.01%     
==========================================
  Files         338      338              
  Lines       59723    59735      +12     
==========================================
+ Hits        45444    45455      +11     
- Misses      14279    14280       +1

@@ -810,8 +810,9 @@ def reset_graph_data(self, *_):
self._update_graph()

def _update_graph(self, reset_view=True, **_):
if self.graph.data is None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_update_graph is called from reset_graph_data and update_density. The former already checks that graph.data is not None. You could add this check to the latter as well instead of having it in _update_graph, and just move the assert in _update_graph to the top.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@jerneju jerneju added this to the 3.8 milestone Nov 22, 2017
@lanzagar
Copy link
Contributor

Not in the scope of this PR (which fixes bugs and we want in the 3.8 release), but maybe for future refactoring

  1. assert not self.plotdata is None should be assert self.plotdata is not None
  2. I fail to see why the above asserts are needed at all (plotdata is never None, even after initialization)

Back to the PR - the fixes work for me and look ok.

@lanzagar lanzagar merged commit 8341764 into biolab:master Nov 27, 2017
@jerneju jerneju deleted the freeviz-fix-hideradius branch November 27, 2017 14:57
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