Fix using wand on Windows, and allow loading of binary blobs #261

roelandschoukens · 2015-10-19T02:06:01Z

I made some changes to make wand work on Python 3.5 on Windows:

Python 3.5 breaks a function needed to load the standard C library
With standard ImageMagick installs the location of the DLLs is not stored in %MAGICK_HOME%, but in the registry.

I also needed to read images from binary blobs, which requires passing the width, height and depth, as well as the actual blob. So I changed the Image() constructor to accept that combination of arguments.

…indows Binary Release Work around issue on Python 3.5 (ctypes.util.find_msvcrt() is broken)

…rs with the filename, file or blob parameter, which is necessary to load raw image data. Added the depth parameter to specify the image depth in raw images

bytes([1, 2, 3]) doesn't have the desired behaviour in Python 2.x

dahlia · 2015-11-14T07:05:59Z

Sorry for late review. Could you add .. versionadded::/.. versionchanged:: directives into docstring? Also please write changelog (docs/changes.rst) as well. The next release would be 0.4.2.

…n setting a fake file name. * allow loading raw pixel data using a file name * add a few extra tests * added version to the docstrings

roelandschoukens · 2015-11-16T01:42:52Z

No problem, I will make the changes.

I have made a few extra changes as well, see the last commit on my fork:

use MagickSetFormat for changing the format of blobs, rather than setting a fake file name.
allow loading raw pixel data using a file name
add a few extra tests
added version to the docstrings

I was also thinking of moving some of that new functionality from the constructor into the read() function.

One outstanding issue is that using ctypes.c_void_p is broken on 64-bit Windows. Sometimes calling a function which returns or accepts such pointer will raise an OverflowError for no apparent reason. I have not found a fix which doesn't break other things, but it means that wand (and probably a lot of other packages) can be used reliably only on 32-bit installations.

dahlia · 2015-11-16T01:58:40Z

wand/api.py

+# yet found a suitable fix for this problem
+# 
+# One proposed solution is subclassing ctypes.c_void_p, but that breaks the
+# assert statements.


How does a subclass of c_void_p break the assert statements? Which asserts?

A subclass is always False in a boolean expression, even if that pointer is NULL. But I just found out there was a signature missing in api.py. With that declaration I no longer get that error.

But that being said, I saw the error before when trying to call MagickSetFormat but I'm not seeing that error anymore. I'll let you know if it comes back.

… that function Add missing test resource

roelandschoukens · 2015-11-24T02:01:41Z

I'm not getting the overflow error anymore. Probably it was caused by the missing definition of MagickDeleteOption in api.py.

dahlia · 2015-11-25T18:57:43Z

The broken build was just fixed in upstream master. Could you rebase it on the current master?

roelandschoukens · 2015-11-26T01:49:18Z

OK, all done.
Coveralls is still showing a slight decrease, but that is unavoidable as a lot of the added code is only executed on Windows.

dahlia · 2015-11-26T15:31:55Z

wand/image.py

@@ -2385,6 +2386,14 @@ class Image(BaseImage):
    .. versionadded:: 0.3.0
       The ``resolution`` parameter.

+    .. versionadded:: 4.2.0


It has to be 0.4.2.

Conflicts: wand/image.py

roelandschoukens · 2015-11-26T21:10:46Z

OK, that's fixed.

dahlia · 2015-11-27T17:52:39Z

CI build was broken due to its code style:

$ flake8 .
./wand/api.py:678:39: E128 continuation line under-indented for visual indent
./wand/api.py:682:39: E128 continuation line under-indented for visual indent

dahlia · 2015-11-27T17:53:10Z

wand/api.py

@@ -650,6 +674,14 @@ class AffineMatrix(ctypes.Structure):
                                      ctypes.c_uint]
    library.MagickSetSize.restype = ctypes.c_int

+    library.MagickSetDepth.argtypes = [ctypes.c_void_p,
+                                      ctypes.c_uint]


./wand/api.py:678:39: E128 continuation line under-indented for visual indent

roelandschoukens · 2015-11-27T22:20:22Z

Yes, I missed those two, sorry about that. The CI build is now passing.

Fix using wand on Windows, and allow loading of binary blobs

dahlia · 2015-11-29T19:03:28Z

Merged! Thanks for your effort. 😄

roelandschoukens · 2015-11-29T20:35:22Z

No problem, thank you for your time reviewing it.

dahlia · 2015-11-29T20:36:35Z

FYI it was included in 0.4.2 release.
On Mon, Nov 30, 2015 at 5:35 AM roelandschoukens notifications@github.com
wrote:

No problem, thank you for your time reviewing it.

—
Reply to this email directly or view it on GitHub
#261 (comment).

roelandschoukens added 7 commits October 7, 2015 10:25

Fix loading the DLL on Windows if ImageMagick was installed using a W…

76962b8

…indows Binary Release Work around issue on Python 3.5 (ctypes.util.find_msvcrt() is broken)

removed extra import

d9cee04

For creating new images allow combining the width and height paramete…

abc0eee

…rs with the filename, file or blob parameter, which is necessary to load raw image data. Added the depth parameter to specify the image depth in raw images

Add a test to read a blob, fix creating empty images

6e671b6

Fix import error in python 2.x

bc110ef

bytes([1, 2, 3]) doesn't have the desired behaviour in Python 2.x

Forgot to import struct

f245f84

Merge remote-tracking branch 'upstream/master'

f9b3065

* use MagickSetFormat for changing the format of blobs, rather tha…

32afee7

…n setting a fake file name. * allow loading raw pixel data using a file name * add a few extra tests * added version to the docstrings

dahlia reviewed Nov 16, 2015
View reviewed changes

Add missing signature to api.py, fixing an OverflowError when calling…

cd4f33f

… that function Add missing test resource

Merge branch 'master' of https://github.com/dahlia/wand

c756ff0

dahlia reviewed Nov 26, 2015
View reviewed changes

roelandschoukens added 2 commits November 27, 2015 10:02

Merge branch 'master' of https://github.com/dahlia/wand

09d866b

Conflicts: wand/image.py

Fix wrong version number in comments

1747cee

Clean up code

7e92182

dahlia reviewed Nov 27, 2015
View reviewed changes

Cleaning up: fix two more bad alignments

4714581

dahlia added a commit that referenced this pull request Nov 29, 2015

Merge pull request #261 from roelandschoukens/master

dd4d2b5

Fix using wand on Windows, and allow loading of binary blobs

dahlia merged commit 4714581 into emcconville:master Nov 29, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix using wand on Windows, and allow loading of binary blobs #261

Fix using wand on Windows, and allow loading of binary blobs #261

roelandschoukens commented Oct 19, 2015

dahlia commented Nov 14, 2015

roelandschoukens commented Nov 16, 2015

dahlia Nov 16, 2015

roelandschoukens Nov 16, 2015

roelandschoukens commented Nov 24, 2015

dahlia commented Nov 25, 2015

roelandschoukens commented Nov 26, 2015

dahlia Nov 26, 2015

roelandschoukens commented Nov 26, 2015

dahlia commented Nov 27, 2015

dahlia Nov 27, 2015

roelandschoukens commented Nov 27, 2015

dahlia commented Nov 29, 2015

roelandschoukens commented Nov 29, 2015

dahlia commented Nov 29, 2015

Fix using wand on Windows, and allow loading of binary blobs #261

Fix using wand on Windows, and allow loading of binary blobs #261

Conversation

roelandschoukens commented Oct 19, 2015

dahlia commented Nov 14, 2015

roelandschoukens commented Nov 16, 2015

dahlia Nov 16, 2015

Choose a reason for hiding this comment

roelandschoukens Nov 16, 2015

Choose a reason for hiding this comment

roelandschoukens commented Nov 24, 2015

dahlia commented Nov 25, 2015

roelandschoukens commented Nov 26, 2015

dahlia Nov 26, 2015

Choose a reason for hiding this comment

roelandschoukens commented Nov 26, 2015

dahlia commented Nov 27, 2015

dahlia Nov 27, 2015

Choose a reason for hiding this comment

roelandschoukens commented Nov 27, 2015

dahlia commented Nov 29, 2015

roelandschoukens commented Nov 29, 2015

dahlia commented Nov 29, 2015