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] Orange: add more informatino on missing compiled libraries #3614

Merged
merged 1 commit into from
Feb 20, 2019

Conversation

robertcv
Copy link
Collaborator

Issue

Fixes #3482
There is unfortunately no straightforward way to add post install for wheels (pypi/packaging-problems#64)

Description of changes

Add additional information if import of compiled libraries failes.

Includes
  • Code changes
  • Tests
  • Documentation

from Orange.data import _variable
except ImportError:
raise ImportError("Compiled libraries cannot be found.\n"
"Try reinstalling the package with: pip install --no-binary Orange3") from None
Copy link
Contributor

Choose a reason for hiding this comment

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

Break this line and then I'm merging. :)

Since you'll modify the commit anyway, do you think it would make sense to import other modules, too? It's probably possible that _variable would exist, but some other module would fail to compile.

In the future, we probably won't remember to add new modules here when we introduce them, but it's still better to have at least those that exist now.

This is just a thought. If you disagree, just break the line and we're done. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added the brake.
If the compile fails pip shows errors. I think the import problems lies somewhere else.
I would therefore argue that either all library work or none does so there is no point in adding all the imports.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant breaking the line because of pylint, not to add a \n to the message, too. But this is OK as well.

@robertcv robertcv force-pushed the fix/compiled_libraries branch from 02eb4ab to 7727809 Compare February 20, 2019 14:56
@codecov
Copy link

codecov bot commented Feb 20, 2019

Codecov Report

Merging #3614 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3614      +/-   ##
==========================================
+ Coverage   84.07%   84.07%   +<.01%     
==========================================
  Files         370      370              
  Lines       67256    67252       -4     
==========================================
- Hits        56544    56542       -2     
+ Misses      10712    10710       -2

@codecov
Copy link

codecov bot commented Feb 20, 2019

Codecov Report

Merging #3614 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3614      +/-   ##
==========================================
+ Coverage   84.07%   84.07%   +<.01%     
==========================================
  Files         370      370              
  Lines       67256    67256              
==========================================
+ Hits        56544    56545       +1     
+ Misses      10712    10711       -1

@janezd janezd merged commit 85da312 into biolab:master Feb 20, 2019
@robertcv robertcv deleted the fix/compiled_libraries branch May 31, 2019 11:30
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.

2 participants