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] Error Reporting: open encode as unicode #2416

Merged
merged 1 commit into from
Aug 23, 2017
Merged

[FIX] Error Reporting: open encode as unicode #2416

merged 1 commit into from
Aug 23, 2017

Conversation

jerneju
Copy link
Contributor

@jerneju jerneju commented Jun 21, 2017

Issue
Description of changes
Includes
  • Code changes
  • Tests
  • Documentation

@jerneju
Copy link
Contributor Author

jerneju commented Jun 21, 2017

@codecov-io
Copy link

codecov-io commented Jun 21, 2017

Codecov Report

Merging #2416 into master will increase coverage by 3.32%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2416      +/-   ##
==========================================
+ Coverage    73.9%   77.22%   +3.32%     
==========================================
  Files         321      325       +4     
  Lines       55993    64896    +8903     
==========================================
+ Hits        41379    50118    +8739     
- Misses      14614    14778     +164

@janezd
Copy link
Contributor

janezd commented Jun 21, 2017

My first question after seeing this PR was --- why?

Is this needed because scheme_to_ows_stream saves as utf-8? Please provide better description of PRs: what they are fixing, how, what is needed to reproduce the bug (if applicable)... to make the reviewers' work easier.

@jerneju jerneju changed the title [FIX] Error Reporting: open encode as unicode [WIP][FIX] Error Reporting: open encode as unicode Jun 23, 2017
@janezd
Copy link
Contributor

janezd commented Aug 22, 2017

Is this still WIP? Plus, can you answer the above question so we can proceed with the PR?

@jerneju
Copy link
Contributor Author

jerneju commented Aug 23, 2017

I have not reproduced this bug yet, so now I am (temporarily?) closing this PR.

@jerneju jerneju closed this Aug 23, 2017
@ales-erjavec
Copy link
Contributor

Run LC_ALL=C orange-canvas
Add some widget to the workflow, rename it to some non ascii name and trigger an exception (in some way):

-------------------------------------------------------------------------------
Error in sys.excepthook:
Traceback (most recent call last):
  File "/usr/local/Cellar/python3/3.5.2_3/Frameworks/Python.framework/Versions/3.5/lib/python3.5/unittest/mock.py", line 1157, in patched
    return func(*args, **keywargs)
  File "/Users/aleserjavec/workspace/orange3/Orange/canvas/application/errorreporting.py", line 229, in handle_exception
    data['_' + F.WIDGET_SCHEME] = f.read()
  File "/Users/aleserjavec/virtual/orange3/bin/../lib/python3.5/encodings/ascii.py", line 26, in decode
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc4 in position 237: ordinal not in range(128)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/aleserjavec/workspace/orange3/Orange/canvas/application/outputview.py", line 228, in __call__
    self.handledException.emit(((exc_type, exc_value, tb), self._canvas))
SystemError: <built-in method emit of PyQt5.QtCore.pyqtBoundSignal object at 0x125cd9558> returned a result with an error set

@ales-erjavec ales-erjavec reopened this Aug 23, 2017
@jerneju
Copy link
Contributor Author

jerneju commented Aug 23, 2017

Now I reproduced this error, tested it and this fix works.

@kernc kernc changed the title [WIP][FIX] Error Reporting: open encode as unicode [FIX] Error Reporting: open encode as unicode Aug 23, 2017
@kernc kernc merged commit d9c030d into biolab:master Aug 23, 2017
@jerneju jerneju deleted the unicodedecode-errorreporting branch August 24, 2017 06:51
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