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

Add Error Reporting #1749

Closed
wants to merge 1 commit into from
Closed

Conversation

prasastoadi
Copy link

Issue

TODO: error reporting

Description of changes

Add error reporting

Includes
  • Code changes
  • Tests
  • Documentation

@CLAassistant
Copy link

CLAassistant commented Nov 13, 2016

CLA assistant check
All committers have signed the CLA.

@codecov-io
Copy link

codecov-io commented Nov 13, 2016

Current coverage is 88.87% (diff: 100%)

Merging #1749 into master will not change coverage

@@             master      #1749   diff @@
==========================================
  Files            82         82          
  Lines          8819       8819          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           7838       7838          
  Misses          981        981          
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 909a119...f4da797

@@ -97,9 +97,9 @@ def load(self, filename):
try:
classifier = pickle.load(open(filename, "rb"))
except pickle.UnpicklingError:
raise # TODO: error reporting
raise pickle.UnpicklingError("Failed to load '{}'.".format(filename))
Copy link
Contributor

Choose a reason for hiding this comment

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

Only one space should appear between raise and argument, here and below..
Please amend your commit.

@janezd
Copy link
Contributor

janezd commented Nov 14, 2016

Thanks for noticing this and for contributing to Orange.

However, the change you've made still raises an exception instead of showing a nicely formatted end-user-readable message, as widgets are supposed to.

The actual fix is short, but requires understanding how error messages in widgets work. For demonstration, I implemented the fix for this bug myself. See the changes in PR #1752.

Can you now find another widget that crashes due to user's fault (like this one) and add a similar fix there?

@janezd janezd closed this Nov 14, 2016
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