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

[BUG] Generators after being cythonized cannot be inspect as generators #6096

Closed
Tiarles opened this issue Mar 19, 2024 · 13 comments
Closed

Comments

@Tiarles
Copy link

Tiarles commented Mar 19, 2024

Describe the bug

Generators that are cythonized cannot be identified as generators when inspect.isgeneratorfunction is used.

Code to reproduce the behaviour:

The code below is used in the package that will be cythonized:

# File src/test_gens/impl.pyx
# Code to be cythonized
def _my_generator():
    value = 6
    print("my_generator:", value)
    yield value
    value = value + 6
    print("my_generator:", value)

and then used by the test

# File test.py
import inspect
from test_gens import my_generator

for i in my_generator():
    print("test:", i)

print("inspect.isgeneratorfunction(main.my_generator):", inspect.isgeneratorfunction(my_generator))

# inspect.isgeneratorfunction(main.my_generator): False

Expected behaviour

Running the same test with the src/test_gens/impl.pyx code in a .py file the return is True, as expected.

OS

Windows

Python version

3.8.4, 3.9.6, 3.10.4, 3.11.4

Cython version

3.0.0

Additional context

No response

@Tiarles
Copy link
Author

Tiarles commented Mar 19, 2024

I initiate the discussion of this issue here.

@da-woods
Copy link
Contributor

It works for me on Cython 3.0.9 with Python 3.11.5.

Have you set the directive # cython: binding = False somewhere? That turns off a lot of introspection and means that inspect.isgeneratorfunction won't work on them. It's on by default though.

@Tiarles
Copy link
Author

Tiarles commented Mar 20, 2024

Fun fact: I found on the repository a comment that was not made by me but never called to my attention:

ext_modules = cythonize(
    ["**/*.pyx"],
    compiler_directives={
        "binding": True,  # so inspect function works properly on cythonized functions
        "language_level": language_level,
    },  # avoid cython warnings
)

So yes, explicitly the biding is set as True.

I look for the line # cython: binding = False or just cython: binding but no place else we define that.

I can try with the most recent Cython version and let you know the outcome.

@Tiarles
Copy link
Author

Tiarles commented Mar 20, 2024

No changes with cython==3.0.9. Let me know if a more explanatory example is needed.

@da-woods
Copy link
Contributor

To clarify, what I tested was

# gen.py
def _my_generator():
    value = 6
    print("my_generator:", value)
    yield value
    value = value + 6
    print("my_generator:", value)

and then tested it with:

# test_gen.py
import gen
import inspect
import os

print(os.path.split(gen.__file__)[1])
print(gen._my_generator)
print(gen._my_generator.__code__.co_flags)
print(inspect.isgeneratorfunction(gen._my_generator))

That gives

gen.py
<function _my_generator at 0x7fa38359cd60>
35
True

Then if I do cythonize -if gen.py and run it again I get

gen.cpython-311-x86_64-linux-gnu.so
<cyfunction _my_generator at 0x7f286ca69e50>
35
True

I don't think your example that you show in the issue is quite what you actually run - for example the module and function names don't match.

@Tiarles
Copy link
Author

Tiarles commented Mar 22, 2024

The only step that I miss explaining is that the my_generator function is built as a package. Cythonized using the follow lines on setup.py file:

ext_modules = cythonize(
    ["**/*.pyx"],
    compiler_directives={
        "binding": True,  # so inspect function works properly on cythonized functions
        "language_level": language_level,
    },  # avoid cython warnings
)

Then, the module is installed and imports the function from the cythonized wheel. I can create a small repository with the example and attach it to our discussion with a tox file for execution.

@Tiarles
Copy link
Author

Tiarles commented Mar 22, 2024

I managed to pack nicely in a repository (cy_wheel_gens_tests).

@da-woods
Copy link
Contributor

The issue is your wrapper in __init__.py which doesn't have anything to do with Cython. Here's a reduced version of your example (no Cython necessary):

def _my_generator():
    yield 1

def my_generator():
    return _my_generator()

import inspect

print(inspect.isgeneratorfunction(my_generator))

This prints False because of the wrapper.

@da-woods da-woods closed this as not planned Won't fix, can't repro, duplicate, stale Mar 22, 2024
@Tiarles
Copy link
Author

Tiarles commented Mar 23, 2024

Aha, I see. I will make a better example because the issue that we are having is on pluggy verification of pytest hooks.

I will investigate more and if persists I will open a new ticket. Thank you for your time.

@Tiarles
Copy link
Author

Tiarles commented Mar 25, 2024

Please, if is possible can you try to take a look at example again ? In the same repository here. I upadted the README.md with the updates made on implementation and on the test.

@da-woods
Copy link
Contributor

Works for me on 3.10 and 3.11:

====================================================== 1 passed in 0.01s ======================================================
  py38: FAIL code 1 (5.00=setup[0.04]+cmd[0.54,2.93,1.04,0.45] seconds)
  py39: FAIL code 1 (4.75=setup[0.01]+cmd[0.51,2.87,0.91,0.44] seconds)
  py310: OK (4.61=setup[0.01]+cmd[0.46,2.81,0.94,0.40] seconds)
  py311: OK (4.04=setup[0.01]+cmd[0.45,2.47,0.89,0.21] seconds)

On 3.8, 3.9 inspect.isfunction requires return isinstance(object, types.FunctionType) which we cannot meet.

The changes in inspect that make it work right came in Python 3.10.6 and 3.11.0. (python/cpython#94050)

There's no way for us to make it work on earlier versions of Python.

@Tiarles
Copy link
Author

Tiarles commented Mar 26, 2024

Understood. Anyway, thank you once more for your time.

@da-woods
Copy link
Contributor

Your other option would be to monkey-patch inspect. I think we do it ourselves inside runtests.py (

def patch_inspect_isfunction():
)

At one point Cython did this specifically for isgenerator in some circumstances. That wouldn't have helped you specifically, although the basic idea could have been used. I think it got removed - possibly semi-mistakenly mainly because it wasn't tested. I don't think Cython should return to doing that in Cython itself though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants