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

Migrate to HPy 0.9.0. #2

Open
wants to merge 2 commits into
base: hpy-9.2.0
Choose a base branch
from
Open

Conversation

fangerer
Copy link

@fangerer fangerer commented May 9, 2023

I've rebased Du Toit's original branch onto branch hpy-9.2.0 of this repo (not sure who created that branch initially).
Then I've migrated to HPy 0.9.0 and this is the corresponding PR.

The tests are not passing but haven't been passing before (making them pass is a separate task).
This is the current test output on my box:

Running selftest:
**********************************************************************
File "/Users/fa/work/repos/pyhandle/pillow-hpy/selftest.py", line 57, in __main__.testimage
Failed example:
    type(im.im) # doctest: +ELLIPSIS
Expected:
    <... '...ImagingCore'>
Got:
    <class 'ImagingCoreOriginal'>
**********************************************************************
File "/Users/fa/work/repos/pyhandle/pillow-hpy/selftest.py", line 78, in __main__.testimage
Failed example:
    im.getextrema()
Exception raised:
    Traceback (most recent call last):
      File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/doctest.py", line 1350, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest __main__.testimage[24]>", line 1, in <module>
        im.getextrema()
      File "/private/var/folders/pn/n5d8qrwj6_z9_jm37l2s245r0000gn/T/venv-ujson_hpy/lib/python3.10/site-packages/PIL/Image.py", line 1342, in getextrema
        extrema.append(self.im.getband(i).getextrema())
    ValueError: band index out of range
**********************************************************************
File "/Users/fa/work/repos/pyhandle/pillow-hpy/selftest.py", line 111, in __main__.testimage
Failed example:
    im.getextrema()
Exception raised:
    Traceback (most recent call last):
      File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/doctest.py", line 1350, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest __main__.testimage[41]>", line 1, in <module>
        im.getextrema()
      File "/private/var/folders/pn/n5d8qrwj6_z9_jm37l2s245r0000gn/T/venv-ujson_hpy/lib/python3.10/site-packages/PIL/Image.py", line 1342, in getextrema
        extrema.append(self.im.getband(i).getextrema())
    ValueError: band index out of range
**********************************************************************
1 items had failures:
   3 of  59 in __main__.testimage
***Test Failed*** 3 failures.
*** 3 tests of 59 failed.
make: *** [install] Error 1

@fangerer fangerer requested review from mattip and DuToitSpies May 9, 2023 08:54
Copy link

@DuToitSpies DuToitSpies left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for making the changes!

@@ -2322,19 +2345,25 @@ static HPy Imaging_getextrema_impl(HPyContext *ctx, HPy self) {
if (status) {
switch (im_self->image->type) {
case IMAGING_TYPE_UINT8:
return HPy_NULL;//HPy_BuildValue(ctx, "II", extrema.u[0], extrema.u[1]);
// return HPy_BuildValue(ctx, "BB", extrema.u[0], extrema.u[1]);

Choose a reason for hiding this comment

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

I think this comment can go - there was an issue while benchmarking in which PyPy didn't like HPy_BuildValue, but it worked fine on CPython and GraalPy, so these HPy_BuildValue comments are just a legacy of then

im->image8));
HPyDef_GET(Imaging_getattr_unsafe_ptrs, "unsafe_ptrs")
static HPy Imaging_getattr_unsafe_ptrs_get(HPyContext *ctx, HPy self, void *closure) {
Imaging im = ImagingObject_AsStruct(ctx, self)->image;

Choose a reason for hiding this comment

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

I remember I had an issue here where casting image to Imaging worked, but using ImagingObject_AsStruct didn't compile. If this compiles, it's fine

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