Update to new conway-polynomials python package by orlitzky · Pull Request #36765 · sagemath/sage

orlitzky · 2023-11-24T19:30:21Z

Fix #32747 by switching to the newly-minted conway-polynomials package on pypi.

IMO sage.databases.conway (which makes the dict immutable by wrapping it in a class) is of dubious value but I've left everything alone for now.

There's now a python package providing a standard interface to this data.

build/pkgs/conway_polynomials/SPKG.rst

mkoeppe · 2023-11-24T19:38:13Z

src/sage/databases/conway.py

            sage: len(c)
-            35352
+            35357
        """


Oh? What happened there?

I was asking myself the same question.

Inflation.

The old package came with preprocessed data that would have been harder to diff against the upstream list for changes. I think it was just a tiny bit out-of-date. Here's the diff:

103a104 > [2,110,[1,1,1,0,0,0,1,0,0,0,1,1,0,1,1,0,0,1,0,0,1,0,0,0,0,1,1,0,1,1,0,0,0,1,0,1,1,1,0,1,0,0,1,1,0,0,0,0,1,0,0,0,0,0,1,1,1,1,1,1,0,0,1,0,1,1,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1]], 219a221 > [3,52,[2,0,1,1,1,2,0,1,2,0,0,0,1,0,2,0,2,2,2,1,2,0,2,1,0,0,2,2,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1]], 222a225,226 > [3,56,[2,2,0,0,0,1,0,2,0,0,0,0,2,1,1,0,0,1,2,2,2,2,0,2,2,1,2,2,0,1,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1]], > [3,57,[1,1,2,2,2,2,1,0,1,1,0,2,1,0,2,2,2,1,0,2,1,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1]], 229a234 > [3,72,[2,2,0,2,1,1,1,2,1,0,2,1,2,1,0,1,0,2,1,1,0,2,1,2,1,2,0,2,0,1,1,1,2,2,1,0,2,2,1,1,0,0,1,2,0,1,1,2,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1]],

I see. Does upstream update the list "often"?

No idea. There's no change history on the site, but the new package ships the upstream file unmodified so at least we can git diff it in the future.

Shipping upstream file unmodified sounds very good to me. I think it is ready.

build/pkgs/conway_polynomials/SPKG.rst

Add back the "conway_polynomials: " prefix in the header to fix the package's inclusion into the TOC.

dimpase · 2023-11-25T00:36:07Z

isn't this duplicating data available from GAP ?

mkoeppe · 2023-11-25T00:36:58Z

We discussed that in #32747.

mkoeppe · 2023-11-25T00:38:42Z

sage -t --long --random-seed=286735480429121101562228604801325644303 sage/databases/all.py  # 1 doctest failed
sage -t --long --random-seed=286735480429121101562228604801325644303 sage/features/__init__.py  # 1 doctest failed

This fixes a failing doctest describing the database.

This fixes a failing doctest for the description of the Conway polynomials database.

mkoeppe

LGTM, thanks for working on this!

orlitzky · 2023-11-25T02:13:21Z

No problem, thanks for the help. I'll work on the others as time permits. We're in the single-digits now for missing Gentoo packages and zero is starting to look possible.

tobiasdiez

Nice, thanks for working on this.

The conda tests are failing, e.g. because of

File "src/sage/combinat/designs/database.py", line 820, in sage.combinat.designs.database.OA_9_135
Failed example:
    OA = OA_9_135()                                                           # needs sage.rings.finite_rings sage.schemes
Exception raised:
    Traceback (most recent call last):
      File "/home/runner/work/sage/sage/src/sage/doctest/forker.py", line 709, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/runner/work/sage/sage/src/sage/doctest/forker.py", line 1144, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.combinat.designs.database.OA_9_135[2]>", line 1, in <module>
        OA = OA_9_135()                                                           # needs sage.rings.finite_rings sage.schemes
      File "/home/runner/work/sage/sage/src/sage/combinat/designs/database.py", line 837, in OA_9_135
        G,B = singer_difference_set(16,2)
      File "/home/runner/work/sage/sage/src/sage/combinat/designs/difference_family.py", line 390, in singer_difference_set
        c = conway_polynomial(p,e*(d+1))
      File "/home/runner/work/sage/sage/src/sage/rings/finite_rings/conway_polynomials.py", line 63, in conway_polynomial
        return R(ConwayPolynomials()[p][n])
      File "/home/runner/work/sage/sage/src/sage/databases/conway.py", line 96, in __init__
        import conway_polynomials
    ModuleNotFoundError: No module named 'conway_polynomials'

So we need to also create a conda feedstock for it.

I think we can also remove the conway_polynomials feature (these failures show that it's presence or absence doesn't really imply anything).

tobiasdiez · 2023-11-25T02:54:51Z

src/sage/features/databases.py

-                            spkg='conway_polynomials',
-                            description="Frank Luebeck's database of Conway polynomials",
-                            type='standard')
+        PythonModule.__init__(


Suggested change

PythonModule.__init__(

super().__init__(

(not that it is that important)

mkoeppe · 2023-11-25T03:07:32Z

Right, conway-polynomials needs to be added to "install-requires" in src/setup.cfg.m4

Adding conda packaging is a downstream activity and not the burden of the PR author

conway_polynomials is a python package now and can be depended upon via the python tooling.

The conway_polynomials package is now a standard python dependency of sagelib and does not need to be detected at runtime.

The feature detection for Conway polynomials was removed; they're always installed via a python package now.

tobiasdiez · 2023-11-25T15:16:38Z

Adding conda packaging is a downstream activity and not the burden of the PR author

How is that a downstream activity if we are the ones offering the package?!
I've now opened conda-forge/staged-recipes#24585 for this, does anyone of you wants to be added as maintainer? Would be nice if a license file could be added as well (sagemath/conway-polynomials#1).

mkoeppe · 2023-11-25T16:21:22Z

Adding conda packaging is a downstream activity and not the burden of the PR author

How is that a downstream activity if we are the ones offering the package?!

Upstream = offers source repo and PyPI package

Downstream = provides Debian, Fedora, OpenSuSE, Arch Linux, Conda, Gentoo, ... packaging.

mkoeppe · 2023-11-25T16:58:52Z

src/sage/features/databases.py

    POLYTOPE_DATA_DIR)


-class DatabaseConwayPolynomials(StaticFile):


Normally I think we would say that this is API and should not be removed without deprecation. But I don't object to removing it

tobiasdiez · 2023-11-27T12:48:58Z

It's now available on conda-forge, so it should work now once you added a conda.txt file.

tobiasdiez

LGTM

orlitzky · 2023-11-28T02:38:49Z

We should update this to v0.8 first but my sage has been occupied all day running gap tests.

github-actions · 2023-11-28T15:04:18Z

Documentation preview for this PR (built with commit 1685d81; changes) is ready! 🎉

sagemathgh-36765: Update to new conway-polynomials python package Fix sagemath#32747 by switching to the newly-minted [conway-polynomials package](https://pypi.org/project/conway-polynomials/0.7/) on pypi. IMO `sage.databases.conway` (which makes the dict immutable by wrapping it in a class) is of dubious value but I've left everything alone for now. URL: sagemath#36765 Reported by: Michael Orlitzky Reviewer(s): François Bissey, Matthias Köppe, Michael Orlitzky, Tobias Diez

orlitzky added 4 commits November 24, 2023 14:22

build/pkgs/conway_polynomials: update to v0.7

fc3e788

src/sage/databases/conway.py: update for conway-polynomials-0.7

90decf7

src/sage/features/databases.py: update for conway-polynomials-0.7

bdf36ab

src/sage/env.py: drop obsolete CONWAY_POLYNOMIALS_DATA_DIR

b801eaa

There's now a python package providing a standard interface to this data.

orlitzky requested review from dimpase, kiwifb, mkoeppe and tobiasdiez November 24, 2023 19:31

orlitzky added c: packages: standard s: needs review labels Nov 24, 2023

mkoeppe reviewed Nov 24, 2023

View reviewed changes

build/pkgs/conway_polynomials/SPKG.rst Outdated Show resolved Hide resolved

mkoeppe reviewed Nov 24, 2023

View reviewed changes

build/pkgs/conway_polynomials/SPKG.rst Outdated Show resolved Hide resolved

orlitzky added 2 commits November 24, 2023 17:00

build/pkgs/conway_polynomials/SPKG.rst: fix header

48d6e9c

Add back the "conway_polynomials: " prefix in the header to fix the package's inclusion into the TOC.

build/pkgs/conway_polynomials/dependencies: add $(PYTHON_TOOLCHAIN)

4b45b63

orlitzky force-pushed the update-conway-polynomials branch from 7a1fada to 4b45b63 Compare November 24, 2023 22:00

orlitzky added 2 commits November 24, 2023 19:46

src/sage/databases/all.py: Luebeck -> Lübeck to match conway.py

a76e0fd

This fixes a failing doctest describing the database.

src/sage/features/__init__.py: Luebeck -> Lübeck to match databases.py

bb55f5e

This fixes a failing doctest for the description of the Conway polynomials database.

mkoeppe approved these changes Nov 25, 2023

View reviewed changes

tobiasdiez suggested changes Nov 25, 2023

View reviewed changes

tobiasdiez added s: needs work and removed s: positive review labels Nov 25, 2023

tobiasdiez mentioned this pull request Nov 25, 2023

CI conda: On pull_request, only run 1 macOS job and 1 Linux job #36694

Merged

5 tasks

orlitzky added 2 commits November 24, 2023 22:48

src/setup.cfg.m4: pull in conway_polynomial's install-requires as deps

c579508

conway_polynomials is a python package now and can be depended upon via the python tooling.

src/sage/features: forego detection of conway_polynomials

a2ab364

The conway_polynomials package is now a standard python dependency of sagelib and does not need to be detected at runtime.

src/sage/databases/conway.py: drop DatabaseConwayPolynomials import

c0dc661

The feature detection for Conway polynomials was removed; they're always installed via a python package now.

mkoeppe reviewed Nov 25, 2023

View reviewed changes

conda now has this package independently of Sage

e6af986

tobiasdiez approved these changes Nov 28, 2023

View reviewed changes

tobiasdiez added s: positive review and removed s: needs review labels Nov 28, 2023

build/pkgs/conway_polynomials: update to v0.8

1685d81

vbraun merged commit dfdd7da into sagemath:develop Dec 14, 2023

github-actions bot removed the s: positive review label Dec 14, 2023

orlitzky deleted the update-conway-polynomials branch January 24, 2024 14:45

		POLYTOPE_DATA_DIR)


		class DatabaseConwayPolynomials(StaticFile):

Uh oh!

Conversation

orlitzky commented Nov 24, 2023

Uh oh!

Uh oh!

mkoeppe Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

kiwifb Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

orlitzky Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

kiwifb Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

orlitzky Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

kiwifb Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dimpase commented Nov 25, 2023

Uh oh!

mkoeppe commented Nov 25, 2023

Uh oh!

mkoeppe commented Nov 25, 2023

Uh oh!

mkoeppe left a comment

Choose a reason for hiding this comment

Uh oh!

orlitzky commented Nov 25, 2023

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

tobiasdiez Nov 25, 2023

Choose a reason for hiding this comment

Uh oh!

mkoeppe commented Nov 25, 2023

Uh oh!

tobiasdiez commented Nov 25, 2023

Uh oh!

mkoeppe commented Nov 25, 2023

Uh oh!

mkoeppe Nov 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasdiez commented Nov 27, 2023

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

orlitzky commented Nov 28, 2023

Uh oh!

github-actions bot commented Nov 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mkoeppe Nov 25, 2023 •

edited

Loading