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] Silhouette Plot: handling memory error #2336

Merged
merged 3 commits into from
Jun 9, 2017
Merged

[FIX] Silhouette Plot: handling memory error #2336

merged 3 commits into from
Jun 9, 2017

Conversation

jerneju
Copy link
Contributor

@jerneju jerneju commented May 23, 2017

Issue

Memory error.

Description of changes
Includes
  • Code changes
  • Tests
  • Documentation

@jerneju
Copy link
Contributor Author

jerneju commented May 23, 2017

self.assertFalse(self.widget.Error.memory_error.is_shown())
with unittest.mock.patch(
"numpy.asarray",
side_effect=MemoryError):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you test this without mocking?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor

Choose a reason for hiding this comment

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

So that we see the exception is caught. Which is not the case if such an exception state arises in e.g. an unguarded C code.

self.assertFalse(self.widget.Error.memory_error.is_shown())
with unittest.mock.patch(
"numpy.asarray",
side_effect=MemoryError):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

GH-2336
"""
from Orange.data import Table
import unittest
Copy link
Contributor

Choose a reason for hiding this comment

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

Why local imports?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgotten to move those two lines up.

@codecov-io
Copy link

codecov-io commented May 24, 2017

Codecov Report

Merging #2336 into master will increase coverage by <.01%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master    #2336      +/-   ##
==========================================
+ Coverage   73.36%   73.36%   +<.01%     
==========================================
  Files         317      317              
  Lines       55586    55607      +21     
==========================================
+ Hits        40778    40797      +19     
- Misses      14808    14810       +2

@jerneju jerneju added this to the 3.4.3 milestone Jun 2, 2017
@nikicc nikicc removed this from the 3.4.3 milestone Jun 3, 2017
@janezd janezd merged commit 4302132 into biolab:master Jun 9, 2017
@jerneju jerneju deleted the memory-silhouette branch June 9, 2017 14:37
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.

5 participants