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

types.CodeType arguments differ from CPython #61

Closed
lesteve opened this issue May 17, 2022 · 4 comments
Closed

types.CodeType arguments differ from CPython #61

lesteve opened this issue May 17, 2022 · 4 comments

Comments

@lesteve
Copy link

lesteve commented May 17, 2022

import types

types.CodeType(
    0,
    0,
    0,
    0,
    0,
    19,
    b"\x06\x004\x00K",
    (None, "test.<locals>.func"),
    (),
    (),
    "/tmp/test.py",
    "func",
    17,
    b"\x02\x00\x00\x01",
    (),
    (),
)

With nogil:

TypeError: an integer is required (got type bytes)

The same snippet works fine with CPython 3.9.

Context: I was trying to run the joblib test suite and one of the error happens when pickling a local function with cloudpickle:

import cloudpickle
import pickle

import types


def test():
    def func():
        pass

    pickled = cloudpickle.dumps(func)
    unpickled = pickle.loads(pickled)

test()
py-tb
Traceback (most recent call last):
  File "/tmp/test.py", line 14, in <module>
    test()
  File "/tmp/test.py", line 12, in test
    unpickled = pickle.loads(pickled)
TypeError: an integer is required (got type bytes)

Side-comment: CPython error are a bit nicer when wrong arguments are passed to types.CodeType for example something like this:

TypeError: code() argument 7 must be bytes, not int

Having similar more informative error messages in nogil would be very useful!

@colesbury
Copy link
Owner

Thanks for the bug report. I'll look into this.

@colesbury
Copy link
Owner

I've made some changes to nogil Python better support cloudpickle and submitted a PR to cloudpickle:

cloudpipe/cloudpickle#470

You can use it immediately by running:

pip install cloudpickle==2.1.0dev0

The above will get the patched version of cloudpickle from the alternative Python package index.

I don't plan to change the signature of types.CodeType. That API isn't stable and changes in nearly every Python version.

I agree about the error messages, but I don't plan to change them either. "nogil" Python is using "argument clinic" for the constructor -- that's also what 3.10+ is using -- 3.9 used PyArg_ParseTuple. The error messages for argument clinic weren't as good in 3.9, but were improved in 3.10. I don't think it's worth back porting those changes.

@ogrisel
Copy link

ogrisel commented May 20, 2022

Thanks @colesbury for the fix. I have integrated it upstream and it's now part of cloudpickle 2.1.0 so you should not need to maintain a forked version of cloudpickle with nogil specific fixes.

We can even work on adding a CI entry on cloudpickle to run its tests against the nogil branch if needed.

@colesbury
Copy link
Owner

@ogrisel - awesome! pip install cloudpickle picks up the latest version (2.1.0) from upstream. It works with the latest docker image and building from source (the pyenv install is a bit out of date).

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

No branches or pull requests

3 participants