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

Exposing acb_theta_all #142

Merged
merged 11 commits into from
Jun 16, 2024
Merged

Exposing acb_theta_all #142

merged 11 commits into from
Jun 16, 2024

Conversation

edgarcosta
Copy link
Member

The only thing missing is that I do not know how to handle flint versions older than 3.1.

@oscarbenjamin
Copy link
Collaborator

We could put this in a separate module that is built conditionally:

diff --git a/meson.build b/meson.build
index 7e941db..150e9f6 100644
--- a/meson.build
+++ b/meson.build
@@ -22,6 +22,9 @@ endif
 # flint.pc was missing -lflint until Flint 3.1.0
 if flint_dep.version().version_compare('<3.1')
   flint_dep = cc.find_library('flint')
+  have_acb_theta = true
+else
+  have_acb_theta = false
 endif
 
 pyflint_deps = [dep_py, gmp_dep, mpfr_dep, flint_dep]
diff --git a/src/flint/types/acb_mat.pyx b/src/flint/types/acb_mat.pyx
index 533bdc7..fff3531 100644
--- a/src/flint/types/acb_mat.pyx
+++ b/src/flint/types/acb_mat.pyx
@@ -20,6 +20,11 @@ from flint.flintlib.arf cimport *
 from flint.flintlib.acb cimport *
 from flint.flintlib.acb_mat cimport *
 
+try:
+    from .acb_theta import acb_mat_theta
+except ImportError:
+    acb_mat_theta = None
+
 cimport cython
 
 cdef acb_mat_coerce_operands(x, y):
@@ -814,3 +819,8 @@ cdef class acb_mat(flint_mat):
         if left:
             return Elist, L
         return Elist, R
+
+   def theta(tau, z, square=False):
+       if acb_mat_theta is None:
+           raise NotImplementedError("acb.theta() needs Flint >= 3.1.0")
+       ...
diff --git a/src/flint/types/meson.build b/src/flint/types/meson.build
index bf8caec..4fd6362 100644
--- a/src/flint/types/meson.build
+++ b/src/flint/types/meson.build
@@ -41,6 +41,10 @@ exts = [
   'fmpz_mpoly',
 ]
 
+if have_acb_theta
+  exts = exts + ['acb_theta']
+endif
+
 py.install_sources(
   pyfiles,
   pure: false,

@edgarcosta
Copy link
Member Author

I will try it out.

@oscarbenjamin
Copy link
Collaborator

Btw are you using the meson build or the setup.py?

Right now python-flint is in a sort of hybrid state where either can be used but my suggestion above would only work with meson.

The simplest way to build a development checkout with meson is to use spin like:

pip install -r requirements-dev.txt
spin install

(set PKG_CONFIG_PATH to find Flint if needed.)

Alternatively rather than spin install you can use:

pip install --no-build-isolation --editable .

Either way that gives you a meson-python editable install. Then every time you run

>>> import flint

or

$ python -m flint.test

it will rebuild python-flint if needed and run the tests.

@edgarcosta
Copy link
Member Author

Thank you. That sped up things. Hopefully, all the tests pass now.
I kept the doctests on acb_mad_theta as having conditional doctests is a pain, and we should only have them in one place.

@@ -80,6 +80,8 @@ def run_doctests(verbose=None):
flint.types.acb_series,
flint.types.dirichlet,
flint.functions.showgood]
if hasattr(flint.types, 'acb_theta'):
modules.append(flint.types.acb_theta)
Copy link
Member Author

Choose a reason for hiding this comment

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

I must be doing something wrong, as I only get 2816 doctests...

Copy link
Collaborator

Choose a reason for hiding this comment

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

hasattr will only work if the module was already imported. I think this needs a try/import.

Copy link
Collaborator

Choose a reason for hiding this comment

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

diff --git a/src/flint/test/__main__.py b/src/flint/test/__main__.py
index 1bd13de..f1dcaf0 100644
--- a/src/flint/test/__main__.py
+++ b/src/flint/test/__main__.py
@@ -80,8 +80,11 @@ def run_doctests(verbose=None):
                flint.types.acb_series,
                flint.types.dirichlet,
                flint.functions.showgood]
-    if hasattr(flint.types, 'acb_theta'):
-        modules.append(flint.types.acb_theta)
+    try:
+        from flint.types import acb_theta
+        modules.append(acb_theta)
+    except ImportError:
+        pass
     results = [doctest.testmod(x) for x in modules]
 #    ffmpz, tfmpz = doctest.testmod(flint._fmpz, verbose=verbose)
  #   failed, total = doctest.testmod(flint.pyflint, verbose=verbose)

@oscarbenjamin
Copy link
Collaborator

I think that doc/source/acb_theta.rst is needed as well.

@oscarbenjamin
Copy link
Collaborator

The docs can be built with spin docs.

@oscarbenjamin
Copy link
Collaborator

Looks good. Thanks!

@oscarbenjamin oscarbenjamin merged commit 33a4daa into flintlib:master Jun 16, 2024
30 checks passed
@oscarbenjamin
Copy link
Collaborator

We should add an automatically generated interface that makes it possible to call any Flint function (gh-54). Then it would be easier to access something like acb_theta without needing any changes in python-flint itself.

@edgarcosta edgarcosta mentioned this pull request Jun 17, 2024
edgarcosta added a commit to edgarcosta/python-flint that referenced this pull request Jul 3, 2024
oscarbenjamin added a commit that referenced this pull request Jul 3, 2024
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