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

Improve error message for kiwisolver import error (DLL load failed) #14316

Merged
merged 3 commits into from
May 30, 2019

Conversation

jonascj
Copy link

@jonascj jonascj commented May 24, 2019

Installing matplotlib with pip on a fresh/clean installation of Windows might fail with an unhelpful ImportError: DLL load failed. One reason for this could be missing Microsoft Visual C++ Redistributable dependencies. This PR adds a check for this condition and provides a more helpful error message if the import fails.

Tested on Windows 10 64bit, Python 3.7.3. The import error is caught and the helpful error message displayed. To test this I was unable to use pip install -e . from within the clone of the repo, because then the missing dependency is already an issue when building matplotlib.ft2font. To work around this I installed matplotlib from pypi pip install matplotlib (which does not perform the build) then replaced my-vevn/Lib/site-packages/matplotlib/__init__.py with my patched __init__.py.

I haven't written any pytest style tests for this, I have close to no idea how to mock the ImportError or simulate the missing dependency.

Thank you @MatthieuDartiailh for your input (nucleic/kiwi#69).

module = importlib.import_module(modname)
except ImportError as error:
if sys.platform == 'win32' and 'DLL' in error.msg:
msg = ('You may be missing MIcrosoft Visual C++ '
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
msg = ('You may be missing MIcrosoft Visual C++ '
msg = ('You may be missing Microsoft Visual C++ '

@cgohlke
Copy link
Contributor

cgohlke commented May 25, 2019

The matplotlib wheels on PyPI include the Microsoft Visual C++ redistributable DLLs. To workaround this issue one could load a matplotlib extension first, e.g.:

diff --git a/lib/matplotlib/__init__.py b/lib/matplotlib/__init__.py
index e0ef418f3..83345af3b 100644
--- a/lib/matplotlib/__init__.py
+++ b/lib/matplotlib/__init__.py
@@ -184,6 +184,8 @@ def compare_versions(a, b):


 def _check_versions():
+    from . import ft2font
+
     for modname, minver in [
             ("cycler", "0.10"),
             ("dateutil", "2.1"),

I don't understand why library versions are checked at runtime. Aren't package managers supposed to handle this based on the package metadata?

@MatthieuDartiailh
Copy link
Contributor

@cgohlke when I looked into this yesterday I read somewhere (cannot remember where now...) that one could not embed the redistributable in the wheel, but it seems wrong.

Do you think I should try to include it in kiwisolver wheels to avoid this issue ? If so do you have a pointer as to how to do that ?

@@ -192,7 +192,17 @@ def _check_versions():
("numpy", "1.11"),
("pyparsing", "2.0.1"),
]:
module = importlib.import_module(modname)
try:
module = importlib.import_module(modname)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is Kiwisolver the only module for which this is true? In which case, maybe check modname to avoid confusing people who get import errors from other modules.

Copy link
Author

Choose a reason for hiding this comment

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

Probably not, but it is the only one I've experienced the problem with.

Also if I'm not mistaken, kiwisolver is not even used by most matplotlibusers. It plays no role in creating basic scatter plots, histograms etc., does it? Maybe one matplotlib could just issue a warning about kiwisolver not being available and load anyway. Someone suggested something like it once: #10386

@jonascj
Copy link
Author

jonascj commented May 25, 2019

The matplotlib wheels on PyPI include the Microsoft Visual C++ redistributable DLLs. To workaround this issue one could load a matplotlib extension first, e.g.:

So the DLL kiwisolver is missing is actually distributed with matplotlib? Then it would be nice if they could be used to prevent this issue.

I don't understand why library versions are checked at runtime. Aren't package managers supposed to handle this based on the package metadata?

Me neither, I too would assume it was pip's job to check dependencies based on metadata. Maybe it has to do with the note around line 100 of __init__.py:

"""
# NOTE: This file must remain Python 2 compatible for the foreseeable future,
# to ensure that we error out properly for existing editable installs.

@tacaswell tacaswell modified the milestones: v3.2.0, v3.1.1 May 26, 2019
@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label May 26, 2019
@tacaswell
Copy link
Member

I don't understand why library versions are checked at runtime. Aren't package managers supposed to handle this based on the package metadata?

Yes, but at least pip will let currently you drive an enviroment to an invalid state (for example if you install a second package which pins to an incompatible version of a dependency or even simple remove a dependency).

Even if we got rid of the version checks here, we unconditionally import kiwisolver later so it would just defer the problem.

Installing matplotlib with pip on a fresh/clean installation of Windows

Is this problem new with mpl 3.1.0? Does in depend on what version of kiwisolver is installed by pip? Do we think this is something about @jonascj 's system or that this is endemic across Windows users?

Maybe one matplotlib could just issue a warning about kiwisolver not being available and load anyway.

But then a major feature (constrained layout) would not work. When this dependency came in we decided to make it a hard dependency because installing it did not seem to be an undue burden and was on net less confusing than documenting and supporting an optional python dependency for functionality (rather external programs or the GUI framework wrappers to enable different kinds of output/embedding).

The usage of kiwisolver is still well contained so we could make it optional if people want to propose that / see what a PR to do that would look, but I do not think that the underlying facts have changed from v2.2 . It seems like this is a packaging issue with kiwisolver (sorry to punt this to you @MatthieuDartiailh ) and should be fixed there rather than on the mpl side.

Given @cgohlke 's comment, I think the reason we are seeing this now is #11980 which unified (and extended) the run time version checking. Previously we were getting lucky with the import order and always importing kiwisolver strictly after matplotlib.ft2font , however that is not the best thing to rely on!

xref: #14303

@tacaswell
Copy link
Member

Given my long comment above I think for 3.1.1 we should either

  1. go with @cgohlke 's suggestion and force a ft2font import as part of checking the versions
  2. defer checking the kiwisolver version until we would otherwise import it in _layoutbox

The "correct" fix is to fix the kiwisolver wheels, but we should do something practical side for now.

@QuLogic
Copy link
Member

QuLogic commented May 26, 2019

Is this problem new with mpl 3.1.0? Does in depend on what version of kiwisolver is installed by pip? Do we think this is something about @jonascj 's system or that this is endemic across Windows users?

Yes, is this a bug in the recently released Matplotlib 3.1.0, or a bug in the semi-recently released kiwisolver 1.1.0?

@jonascj
Copy link
Author

jonascj commented May 26, 2019

Is this problem new with mpl 3.1.0? Does in depend on what version of kiwisolver is installed by pip? Do we think this is something about @jonascj 's system or that this is endemic across Windows users?

Yes, is this a bug in the recently released Matplotlib 3.1.0, or a bug in the semi-recently released kiwisolver 1.1.0?

I would say it is a general issue, it did a fresh install of Windows 10 64bit version 1809 and 1903 several times over on three laptops. Every time the same issue with identical version of python (3.7.3 both 32bit and 64bit, both installed per user and systemwide) and the most recent PyPi version of matplotlib. Also tried installed kiwisolver on its own, same issue. This DLL Load issue is because of kiwisolver.

The mentioned version of python and matplot lib installs just fine on some other Win10 laptops I had access to, but these laptop have had tons of other software installed (Adobe this and that, Unity, Chrome, Firefox etc.) so they likely just have the Microsoft Visual C++ redist DLL's from some other piece of software.

I tested the same version

@jonascj
Copy link
Author

jonascj commented May 26, 2019

Is this problem new with mpl 3.1.0? Does in depend on what version of kiwisolver is installed by pip? Do we think this is something about @jonascj 's system or that this is endemic across Windows users?

@tacaswell see my answer above. I would say it is problem for all windows users. The only reason it will seldom be seen is that it is a rare thing to do a fresh install of Windows 10 and as the first thing install python+matplotlib.

Let me know if you want further testing to confirm this (and if so, what testing, other version of kiwi / matplotlib?). I am not a Windows user myself, but forced to deal with a lot of windows day-to-day. So I have plenty of test material.

@MatthieuDartiailh
Copy link
Contributor

I can re-package kiwi (with the help of @matthew-brett as usual) but it seems a bit wasteful to distribute Microsoft redistributable with everything package requiring the c++ runtime (the way matplotlib seem to be doing).
conda has a saner approach to the problem and allow declaring that a package depends on the c++ runtime. Would it make sense to create a PyPI package for the redistributable on which kiwi could depend ?This way there would be at most one extra copy. It is allowed by Microsoft licence ? @cgohlke @matthew-brett

@matthew-brett
Copy link
Contributor

At the moment we don't have a good system in pip for installing not-Python packages, but we could look to see if we can hack round it.

@zooba - are we allowed to make a pip package for the MS C++ runtime? Or do you (as Microsoft) have to do that? Have y'all already done that?

@cgohlke
Copy link
Contributor

cgohlke commented May 26, 2019

to distribute Microsoft redistributable with everything package requiring the c++ runtime (the way matplotlib seem to be doing)

That's the recommended way and is used by Matplotlib, Pandas, Scipy, pyzmq, numpy+mkl...

Probably the easiest solution for kiwi (not generally recommended though) is to statically link the C/C++ runtime library. Set the DISTUTILS_USE_SDK=1 environment variable and explicitly initialize the VC compiler (e.g. "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvarsall.bat" amd64 ) before building the wheel. IIRC this should work for Python3.5+.

@MatthieuDartiailh
Copy link
Contributor

By curiosity when packaging the Windows redistributable with the library, will newer redistributable installed "normally" be picked up first ?

I found https://github.com/MacPython/scipy-wheels/blob/master/appveyor.yml, that include the redistributable in scipy. Should I follow that example to re-package windows wheel ?

@tacaswell
Copy link
Member

We also would not see this problem on appveyor because we install enough stuff to build Matplotlib so we have the dlls available.

@jonascj Can you verify that @cgohlke 's suggestion works on your machine and push that to this branch instead?

@jonascj
Copy link
Author

jonascj commented May 28, 2019

@tacaswell You mean try this patch and see if it mitigates the problem?

diff --git a/lib/matplotlib/init.py b/lib/matplotlib/init.py
index e0ef418f3..83345af3b 100644
--- a/lib/matplotlib/init.py
+++ b/lib/matplotlib/init.py
@@ -184,6 +184,8 @@ def compare_versions(a, b):
def _check_versions():

  • from . import ft2font
  • for modname, minver in [
    ("cycler", "0.10"),
    ("dateutil", "2.1"),

@cgohlke
Copy link
Contributor

cgohlke commented May 28, 2019

One way to reproduce this issue on a Windows 10 system that already has the Visual C++ Redistributable Package installed is to install Python and matplotlib in a Windows Sandbox.

curl -o python-3.7.3-amd64.exe https://www.python.org/ftp/python/3.7.3/python-3.7.3-amd64.exe
python-3.7.3-amd64.exe /passive
py -m pip install matplotlib
py -c"import matplotlib"

@tacaswell
Copy link
Member

@jonascj Yes

@jonascj
Copy link
Author

jonascj commented May 28, 2019

@tacaswell I'll do this later today or tomorrow.

@cgohlke Interesting, I will however just test it in a virtual machine (restored to a snapshot taken right after completing the Windows 10 installation).

Jonas Camillus Jeppesen added 2 commits May 28, 2019 22:06
…mportError: DLL load failed'"

This reverts commit f14ffb1.
…h otherwise fails with 'ImportError: DLL load failed'
@jonascj
Copy link
Author

jonascj commented May 28, 2019

@cgohlke solutions (importing ft2font before other imports) solves the ImportError: DLL load failed on my Windows test setup.

I've reverted my previous commit (in my own feature branch) and committed this new fix/patch. Let me know if this is not the way to do it, then I'll try to do it another way.

@tacaswell
Copy link
Member

Squashing and force-pushing the first two commits out of existence would be marginally better but we can also squash-merge it.

Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

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

This is the minimal pragmatic solution.

@jonascj
Copy link
Author

jonascj commented May 28, 2019

Squashing and force-pushing the first two commits out of existence would be marginally better but we can also squash-merge it.

So you would have preferred it if I just rewrote history by not having the original commit + revert commit? I'm not sure my git-skills allow me to do that at this point ...

@dstansby
Copy link
Member

@jonascj don't worry, I can squash-merge it so everything gets squashed into one commit.

@dstansby dstansby merged commit a2cb4ee into matplotlib:master May 30, 2019
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request May 30, 2019
@jonascj
Copy link
Author

jonascj commented May 30, 2019

@jonascj don't worry, I can squash-merge it so everything gets squashed into one commit.

I didn't, it was just a question to improve my future contributions. Many a place you can read that rewriting history (i.e. resetting and removing commits, only to make new ones) is bad if the history has already been published/pushed. I was in doubt whether that applied in this case. I suppose not since they were only pushed to my fork...

@dstansby
Copy link
Member

In general I think pushing/pulling to a non-merged personal branch is fine, but we definitely don't squash or force push anything on to any of the matplotlib branches, so the history is always preserved there.

timhoffm added a commit that referenced this pull request May 30, 2019
…316-on-v3.1.x

Backport PR #14316 on branch v3.1.x (Improve error message for kiwisolver import error (DLL load failed))
@tacaswell
Copy link
Member

@dstansby is exactly right, force-pushing to your branches is ok as that is "scratch space" and we may not want to preserve every thing you did along the way into the main history. On the other hand the matplotilb/matplotlib branches are the "branches of record" and once we publish to those we should not re-write history.

Thanks for your work on this @jonascj ! Hopefully we will here from you again!

As always, thank you @cgohlke for being the windows wheels magician!

@frank-yifei-wang
Copy link

frank-yifei-wang commented Nov 6, 2019

Same issue on my machine (Win10 64-bit, Conda 4.7.12, Python 3.7.3.final.0) - import matplotlib works fine but import matplotlib.pyplot as plt throws the error.

Spent hours trying different methods found from GitHub/StackOverflow with no luck. Finally fixed the issue by downgrading to matplotlib 3.0.3 ("older stable version" as stated by the official website).

conda uninstall matplotlib
conda install matplotlib==3.0.3

@timhoffm
Copy link
Member

timhoffm commented Nov 6, 2019

Which matplotlib version have you been having trouble with?

@tacaswell
Copy link
Member

@frank-wang-yifei please report this to anaconda (or conda-forge if you are using their packages). This is almost certainly a packaging problem, not something we can fix in Matplotilb.

@frank-yifei-wang
Copy link

@tacaswell Reported. Thanks!

@frank-yifei-wang
Copy link

@frank-wang-yifei please report this to anaconda (or conda-forge if you are using their packages). This is almost certainly a packaging problem, not something we can fix in Matplotilb.

@timhoffm If you mean my matplotlib, at first I was trying to install version 3.1.1.

@zxdawn
Copy link

zxdawn commented Nov 12, 2019

@tacaswell @frank-wang-yifei You can try this method: conda uninstall matplotlib --> conda clean --all --> pip install matplotlib. Then import matplotlib.pyplot as plt should work fine and the version of matplotlib is 3.1.2.

@frank-yifei-wang
Copy link

@tacaswell @frank-wang-yifei You can try this method: conda uninstall matplotlib --> conda clean --all --> pip install matplotlib. Then import matplotlib.pyplot as plt should work fine and the version of matplotlib is 3.1.2.

Thanks for the tip. At first I was concerned about mixing conda and pip packages. But next time I might give this a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants