GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Exposing acb_theta_all by edgarcosta · Pull Request #142 · flintlib/python-flint

Conversation

@edgarcosta

@edgarcosta edgarcosta commented Jun 14, 2024

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@oscarbenjamin

oscarbenjamin commented Jun 14, 2024

Copy link
Copy Markdown
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,
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@edgarcosta

edgarcosta commented Jun 14, 2024

Copy link
Copy Markdown
Member Author

I will try it out.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@oscarbenjamin

oscarbenjamin commented Jun 14, 2024

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@edgarcosta
edgarcosta force-pushed the acb_theta branch from 7a71475 to 87e7fc0 Compare June 14, 2024 20:58
@edgarcosta

edgarcosta commented Jun 14, 2024

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

edgarcosta
edgarcosta commented Jun 14, 2024
View reviewed changes
Comment thread src/flint/test/__main__.py Outdated
flint.types.dirichlet,
flint.functions.showgood]
if hasattr(flint.types, 'acb_theta'):
modules.append(flint.types.acb_theta)

@edgarcosta edgarcosta Jun 14, 2024

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

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...

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@oscarbenjamin oscarbenjamin Jun 14, 2024

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@oscarbenjamin oscarbenjamin Jun 14, 2024

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

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)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@oscarbenjamin

oscarbenjamin commented Jun 14, 2024

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@oscarbenjamin

oscarbenjamin commented Jun 14, 2024

Copy link
Copy Markdown
Collaborator

The docs can be built with spin docs.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@edgarcosta
edgarcosta force-pushed the acb_theta branch from 7112950 to 9b73fda Compare June 15, 2024 15:42
edgarcosta added 9 commits June 15, 2024 17:00
@edgarcosta
initial commit to acb_theta via rst_to_pxd.py
c266d14
@edgarcosta
Wrapping acb_theta_all
bf2bca7
@edgarcosta
adding documentation
0450b6f
@edgarcosta
making a new module for acb_theta
8379c5e
@edgarcosta
add acb_theta to doctests
f59c5fe
@edgarcosta
return a matrix instead of a tuple
8c74c77
@edgarcosta
fix doctests
fbabf32
@edgarcosta
documentation
0cae160
@edgarcosta
one more example, and now using showgood
d9a0273
@edgarcosta
edgarcosta force-pushed the acb_theta branch from 9b73fda to d9a0273 Compare June 15, 2024 21:51
edgarcosta added 2 commits June 15, 2024 17:56
@edgarcosta
fix doctest
920d48d
@edgarcosta
use ctx.default()
778de4b
@edgarcosta
edgarcosta force-pushed the acb_theta branch from 3489f70 to 778de4b Compare June 16, 2024 04:23
@oscarbenjamin

oscarbenjamin commented Jun 16, 2024

Copy link
Copy Markdown
Collaborator

Looks good. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@oscarbenjamin
oscarbenjamin merged commit 33a4daa into flintlib:master Jun 16, 2024
@oscarbenjamin

oscarbenjamin commented Jun 16, 2024

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@edgarcosta edgarcosta mentioned this pull request Jun 17, 2024
Bump release #145
Closed
edgarcosta added a commit to edgarcosta/python-flint that referenced this pull request Jul 3, 2024
@edgarcosta
expand flintlibgh-142 line
a5c4ca0
oscarbenjamin added a commit that referenced this pull request Jul 3, 2024
@oscarbenjamin
Merge pull request #157 from edgarcosta/patch-1
430b8e5
expand gh-142 line
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oscarbenjamin oscarbenjamin oscarbenjamin left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@edgarcosta @oscarbenjamin
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.