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

Fix cython syntax, add tests #481

Merged
merged 12 commits into from
Jul 9, 2024
Merged

Conversation

bjodah
Copy link
Contributor

@bjodah bjodah commented May 27, 2024

See gh-480 and cython/cython#6218

Fixes #480
Fixes #483

@bjodah
Copy link
Contributor Author

bjodah commented May 31, 2024

added 7472cce since building with cython master gives:

      Error compiling Cython file:
      ------------------------------------------------------------
      ...
      
          def __int__(self):
              return int(float(self))
      
          def __long__(self):
              return long(float(self))
                     ^
      ------------------------------------------------------------
      
      symengine_wrapper.pyx:1208:15: undeclared name not builtin: long
      gmake[2]: *** [symengine/lib/CMakeFiles/symengine_wrapper.dir/build.make:76: symengine/lib/symengine_wrapper.cpp] Error 1
      gmake[1]: *** [CMakeFiles/Makefile2:112: symengine/lib/CMakeFiles/symengine_wrapper.dir/all] Error 2
      gmake: *** [Makefile:136: all] Error 2
      error: error building project
      [end of output]

@bjodah
Copy link
Contributor Author

bjodah commented May 31, 2024

Added dd3dc6e since it fixes gh-483

The flint.types.nmod.nmod return type is causing some problems for me locally. I don't think we have a configuration in our CI which actually tests with https://github.com/flintlib/python-flint installed? I guess we should add one such config too.

@oscarbenjamin
Copy link

I think that the premise of this test is incorrect:

a = sympy.FF(7)(3)
b = sympify(a)

It should be a TypeError to sympify a domain element.

Many other domain/polys types in SymPy inherit CantSympify to enforce this:

$ git grep CantSympify sympy/polys/
sympy/polys/fields.py:from sympy.core.sympify import CantSympify, sympify
sympy/polys/fields.py:class FracElement(DomainElement, DefaultPrinting, CantSympify):
sympy/polys/polyclasses.py:from sympy.core.sympify import CantSympify
sympy/polys/polyclasses.py:class DMP(CantSympify):
sympy/polys/polyclasses.py:class DMF(PicklableWithSlots, CantSympify):
sympy/polys/polyclasses.py:class ANP(CantSympify):
sympy/polys/rings.py:from sympy.core.sympify import CantSympify, sympify
sympy/polys/rings.py:class PolyElement(DomainElement, DefaultPrinting, CantSympify, dict):

I consider it an oversight that ModularInteger does not inherit CantSympify. It isn't possible to enforce for ZZ and QQ but for every other domain we can make sympification fail.

@isuruf isuruf merged commit 9ce6c5a into symengine:master Jul 9, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants