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] binding=True causes a mysterious import error #4775

Closed
kwankyu opened this issue May 9, 2022 · 4 comments
Closed

[BUG] binding=True causes a mysterious import error #4775

kwankyu opened this issue May 9, 2022 · 4 comments

Comments

@kwankyu
Copy link

kwankyu commented May 9, 2022

This issue arose while we tried to turn on binding=True cython compiler option in SageMath 9.6.rc3:

https://trac.sagemath.org/ticket/26254#comment:64

The function call_registered_function() is defined in the file src/sage/symbolic/pynac_function_impl.pxi, which is included into src/sage/symbolic/expression.pyx by include "pynac_function_impl.pxi".

Then the statement from .expression import call_registered_function in the global scope of src/sage/symbolic/function.pyx throws the exception

ImportError: cannot import name call_registered_function

This may be a bug on cython since we don't have this exception with the default binding=False.

SageMath 9.6.rc3 uses python-3.10.3 and cython-0.29.28.

@da-woods
Copy link
Contributor

da-woods commented May 9, 2022

To me this looks like a circular import issue possible? From the sage issue:

File "sage/symbolic/expression.pyx", line 1, in init sage.symbolic.expression
    # -*- coding: utf-8 -*-
  File "sage/symbolic/function.pyx", line 144, in init sage.symbolic.function
    from .expression import (

expression import function which imports expression (some of these imports may be generated by cimport though - I'm not sure).

With binding=False functions are defined in PyModuleDef. I think that means they exist very early in the module import process before the main module init function has run, and therefore you may get away with it. With binding=True they're generated in the module init function.

Therefore I provisionally don't think it's a Cython bug - it looks a lot like code that wouldn't work in Python either

@da-woods
Copy link
Contributor

da-woods commented May 9, 2022

I think if you remove the line

from sage.symbolic.function cimport SymbolicFunction

it'll work. You don't look to actually use the cimport so you're losing nothing by removing it.

However I can't work out how the code is working at all with or without binding. At least on a simplified test locally it makes no difference if it's enabled for not.

@kwankyu
Copy link
Author

kwankyu commented May 10, 2022

To me this looks like a circular import issue possible?

Right. There was a circular import. We could fix the problem by changing the global imports to local imports. Your explanation below confirms that we did the right thing.

With binding=False functions are defined in PyModuleDef. I think that means they exist very early in the module import process before the main module init function has run, and therefore you may get away with it. With binding=True they're generated in the module init function.

So binding=True simply revealed the existing circular import. Thank you for the explanation!

@da-woods
Copy link
Contributor

So binding=True simply revealed the existing circular import. Thank you for the explanation!

Possibly... I tried to make a cut-down example to replicate it myself and couldn't make it work with either version!

Anyway - hopefully you have enough information to fix it... I'll close this issue I think

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