#30332 closed enhancement (fixed)

Merge sage_brial into sagelib

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.2
Component: packages: standard Keywords:
Cc: fbissey, mjo, tscrim Merged in:
Authors: François Bissey Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 91fe5e1 (Commits, GitHub, GitLab) Commit: 91fe5e1c34a3d0b0e4a2960119aa313ad51e687e
Dependencies: Stopgaps:

Status badges

Description

As discussed in #29369 and #28918.

Change History (52)

comment:1 Changed 22 months ago by fbissey

I propose to copy the current sage-brial code under sage/rings/polynomial. This is based on the fact all (but one) imports of brial happen there.

$ grep -r brial src/sage/*
src/sage/rings/polynomial/pbori.pyx:# distutils: libraries = brial brial_groebner M4RI_LIBRARIES LIBPNG_LIBRARIES
src/sage/rings/polynomial/pbori.pyx:    sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:        #from brial.interpolate import interpolate_smallest_lex
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import BooleanMonomialMonoid, BooleanMonomial
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid, BooleanMonomial
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleSet
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import BooleanPolynomial
src/sage/rings/polynomial/pbori.pyx:        from brial.parallel import _encode_polynomial
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleSet
src/sage/rings/polynomial/pbori.pyx:        from brial import red_tail
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:        from brial.gbcore import groebner_basis
src/sage/rings/polynomial/pbori.pyx:        from brial import red_tail
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import BooleSet
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleSet
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleSet
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import BooleanPolynomialVector
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanPolynomialVector
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanPolynomialVector
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanPolynomialVector
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanPolynomialVector
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanPolynomialVector
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanPolynomialVector
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import GroebnerStrategy
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import GroebnerStrategy
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import groebner_basis
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import GroebnerStrategy
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanPolynomialVector
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import GroebnerStrategy
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import groebner_basis
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import GroebnerStrategy
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import GroebnerStrategy
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import GroebnerStrategy
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleanMonomialMonoid
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import map_every_x_to_x_plus_one
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import zeros
src/sage/rings/polynomial/pbori.pyx:        sage: from brial.interpolate import *
src/sage/rings/polynomial/pbori.pyx:        sage: from brial.interpolate import *
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import ll_red_nf_redsb
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import ll_red_nf_noredsb
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import ll_red_nf_noredsb_single_recursive_call
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import if_then_else
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import top_index
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import substitute_variables
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import substitute_variables
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import random_set, set_random_seed
src/sage/rings/polynomial/pbori.pyx:        sage: from brial import random_set, set_random_seed
src/sage/rings/polynomial/pbori.pyx:# todo: merge with pickling from brial.parallel
src/sage/rings/polynomial/pbori.pyx:# todo: merge with pickling from brial.parallel
src/sage/rings/polynomial/pbori.pyx:    from brial.parallel import _decode_polynomial
src/sage/rings/polynomial/pbori.pyx:# todo: merge with pickling from brial.parallel
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleConstant
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleConstant
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleConstant
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleConstant
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleConstant
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleConstant
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleConstant
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import BooleConstant
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/pbori.pyx:            sage: from brial import *
src/sage/rings/polynomial/multi_polynomial_sequence.py:        from brial import gauss_on_polys
src/sage/rings/polynomial/multi_polynomial_sequence.py:        from brial.ll import eliminate,ll_encode,ll_red_nf_redsb
src/sage/rings/polynomial/multi_polynomial_sequence.py:            from brial.interred import interred as inter_red
src/sage/sat/converters/__init__.py:from brial.cnf import CNFEncoder as PolyBoRiCNFEncoder

but other suggestions based on best practices will be given the priority.

comment:2 Changed 22 months ago by mkoeppe

So there's a Cython interface sage.libs.polybori, then a Python library brial, and also sage.rings.polynomial.pbori? What is depending on what?

comment:3 Changed 22 months ago by fbissey

I thought that sage-brial depends on sage.libs.polybori but that may not be the case. It imports sage.all and specifically sage.rings.polynomial.pbori and that's the only sage specific bits it imports.

sage.rings.polynomial.pbori itself relies on sage.libs.polybori (through sage.libs.polybori.decl) and both need the C++ brial library.

Does that answer your question?

comment:4 Changed 22 months ago by mkoeppe

Thanks. I agree that sage.rings.polynomial seems to be a good place for the Python code from sage-brial. Perhaps it would be best to create a package sage.rings.polynomial.brial and to move the current pbori.pyx inside it (leaving deprecated reimport behind). There are only a few places in Sage where things are imported from sage.rings.polynomial.pbori and they could be easily update. Or, if you don't mind keep the old name pbori, the same could be done with sage.rings.polynomial.pbori instead of ...brial.

One thing to keep in mind is the context of the planned modularization of sagelib. In Sage 9.3 I would hope to be able to create a separate distribution (distutils package) sage-brial, which would then package sage.rings.polynomial.{brial,pbori}, sage.libs.polybori, sage.libs.fes, sage.crypto.boolean_function -- everything that has a compile-time dependency on libbrial. This will make it possible to separately compile sagelib on systems without brial.

comment:5 follow-up: Changed 22 months ago by fbissey

On the legal side, the code is GPL2.0 or later. Although I will need to fix the license on github. It seems I changed it from 2+ to 2 carelessly back in 2017 (probably by copying a template). I obviously had no rights to do that.

The only thing I may need guidance with is the deprecated re-import. Otherwise moving stuff, including pbori.{pyx,pxd}, in sage.rings.polynomial.pbori seems to be a good idea and hopefully will help with modularization.

comment:6 in reply to: ↑ 5 ; follow-up: Changed 22 months ago by mkoeppe

Replying to fbissey:

On the legal side, the code is GPL2.0 or later. Although I will need to fix the license on github. It seems I changed it from 2+ to 2 carelessly back in 2017 (probably by copying a template). I obviously had no rights to do that.

I think GitHub takes the position that there is no such thing as "GPL 2 only": That the license includes "... or any later version" no matter whether you write these words in the files or not.

Last edited 22 months ago by mkoeppe (previous) (diff)

comment:7 in reply to: ↑ 6 Changed 22 months ago by fbissey

Replying to mkoeppe:

Replying to fbissey:

On the legal side, the code is GPL2.0 or later. Although I will need to fix the license on github. It seems I changed it from 2+ to 2 carelessly back in 2017 (probably by copying a template). I obviously had no rights to do that.

I think GitHub takes the position that there is no such thing as "GPL 2 only": That the license includes "... or any later version" no matter whether you write these words in the files or not.

Very nice of them. But they are not lawyers and I think the "LICENSE" file should be explicit for people who look for these subtle clarification (distros). The GPL FAQ https://www.gnu.org/licenses/old-licenses/gpl-2.0-faq.html#VersionTwoOrLater seem to imply that you should explicitly state it.

comment:8 Changed 22 months ago by fbissey

Tests included inside sage-brial will have to be moved to sage doctesting. More generally it will have to be documented properly as much as possible. This will be so much fun.

It looks like that will take much more than just drag and drop in place with a few cosmetic changes.

comment:9 Changed 22 months ago by mkoeppe

I suppose you could prepare the change to using the sage doctester first, before merging into Sage.

comment:10 follow-up: Changed 22 months ago by mkoeppe

Given that sage_brial has no Cython bits, I think it could in fact be removed from the dependencies of sagelib, as it won't really be needed while installing sagelib, only later for docbuild and doctest.

comment:11 in reply to: ↑ 10 Changed 22 months ago by fbissey

Replying to mkoeppe:

Given that sage_brial has no Cython bits, I think it could in fact be removed from the dependencies of sagelib, as it won't really be needed while installing sagelib, only later for docbuild and doctest.

Technically correct.

comment:12 Changed 22 months ago by fbissey

  • Branch set to public/merge_sage_brial
  • Commit set to 884ca6663e3e18dd284774b1a3f51b30fdb50882

This is not ready for review. I am just pushing my work in progress so it is not just on my workstation, it is visible and open to others contribution.

There is a lot to do:

  1. update paths
  2. convert docstrings and tests to sage framework

This may take some time and I doubt it will be ready in time for 9.2 but we can only try.


New commits:

884ca66WIP: move sage-brial files and pbori.{pxd,pyx} in sage/rings/polynomial/pbori/

comment:13 Changed 22 months ago by git

  • Commit changed from 884ca6663e3e18dd284774b1a3f51b30fdb50882 to e7653a3417bf7657cfdb65a4685b74284f644b6d

Branch pushed to git repo; I updated commit sha1. New commits:

f947293replace "from sage.rings.polynomial.pbori" with "from sage.rings.polynomial.pbori.pbori"
e7653a3Replace "from brial" with "from sage.rings.polynomial.pbori.brial"

comment:14 Changed 22 months ago by git

  • Commit changed from e7653a3417bf7657cfdb65a4685b74284f644b6d to 2226097a9bf79f4b9c9675fc9fe73ffd043819e6

Branch pushed to git repo; I updated commit sha1. New commits:

2226097src/sage/rings/polynomial/pbori/__init__.py: Draft of lazy_import call

comment:15 Changed 21 months ago by git

  • Commit changed from 2226097a9bf79f4b9c9675fc9fe73ffd043819e6 to d0bc7863a60366121688bb09f64af3618d3e1edc

Branch pushed to git repo; I updated commit sha1. New commits:

d0bc786transform python test into sage tests - step one.

comment:16 Changed 21 months ago by fbissey

I am currently converting docstrings :( I am also doing some selective code deletion as I come along across it. There is a module for memory usage, it is imported once but never used and even less tested. So, I am removing it. It will be worth another look after that first pass.

comment:17 Changed 21 months ago by git

  • Commit changed from d0bc7863a60366121688bb09f64af3618d3e1edc to bdf1fecd4a5532f09abf6cb204dfeb705dfd253e

Branch pushed to git repo; I updated commit sha1. New commits:

ae30cf3Basic conversion to sage doctrings/doctesting. Remove original copyright/license functions.
4c0d80fSome basic docstring in general_boolean_polynomial.py - remove memusage.py and its only import.
bdf1fecFirst pass in converting to docstring and doctests. It is quite likely a lot of doctests are broken at this stage.

comment:18 Changed 21 months ago by git

  • Commit changed from bdf1fecd4a5532f09abf6cb204dfeb705dfd253e to 2e94a7725ea5018c6cfd8cd291df80bcf0c7a846

Branch pushed to git repo; I updated commit sha1. New commits:

2e94a77Merge branch 'develop' into brial-merging

comment:19 Changed 21 months ago by fbissey

I suspect the building of the html doc will break at this stage. If it doesn't the output may very well be useless. Once I figure that out, I'll do the doctesting to see what's broken. I am expecting a lot of breaking in the newly imported tests.

comment:20 Changed 21 months ago by git

  • Commit changed from 2e94a7725ea5018c6cfd8cd291df80bcf0c7a846 to 4e3ed8850f451ee6f65526434397cc880cd38f0c

Branch pushed to git repo; I updated commit sha1. New commits:

4e3ed88one too many pbori.

comment:21 Changed 21 months ago by git

  • Commit changed from 4e3ed8850f451ee6f65526434397cc880cd38f0c to 563e959d6dcf27f9352f5ae3590b2b35148fe2bb

Branch pushed to git repo; I updated commit sha1. New commits:

563e959add miising import of lazy_import

comment:22 Changed 21 months ago by git

  • Commit changed from 563e959d6dcf27f9352f5ae3590b2b35148fe2bb to 408824d44f039c51ba2ab8c9552bf2a0f8cfedcc

Branch pushed to git repo; I updated commit sha1. New commits:

7cb8ba1remove excessive .brial from import. The initial replacement was wrong.
408824dFix building of the pbori interface documentation. No "brial" documentation is buiilt as far as I can tell.

comment:23 Changed 21 months ago by git

  • Commit changed from 408824d44f039c51ba2ab8c9552bf2a0f8cfedcc to 443a3a8d5d3ba3b05db9312f33788d845836808d

Branch pushed to git repo; I updated commit sha1. New commits:

443a3a8Remove addition.py - nothing from this file is used and tests failures point to the file being obsolote.

comment:24 Changed 21 months ago by git

  • Commit changed from 443a3a8d5d3ba3b05db9312f33788d845836808d to 167ef09e38e16eee05a7c278c47558772666e6e8

Branch pushed to git repo; I updated commit sha1. New commits:

167ef09migrate xrange (unsupported in python3) to range

comment:25 Changed 21 months ago by fbissey

At this point documentation builds and is identical to the old one. doctesting gets a lot of failures. There is still quite a bit of cleaning to be done, including removing files that should have been trashed long ago.

comment:26 Changed 21 months ago by fbissey

More joy! I found a subtle coding error while fixing the test suite. I guess it won't have an impact on the rest of sage because that code path just error-ed out when taken. So it is probably a dead code path anyway.

comment:27 Changed 21 months ago by git

  • Commit changed from 167ef09e38e16eee05a7c278c47558772666e6e8 to e25dd810b818d1543b66313d608ca33d93341b76

Branch pushed to git repo; I updated commit sha1. New commits:

afdbb9dfix doctests in blocks.py
ec4cddbremove check_claims.py which appears to be a standalone script.
d53714fremove another script
72f8a6aFix doctest and code in cnf.py
e25dd81Fix pbori related doctests and code in multi_polynomial_sequence

comment:28 Changed 21 months ago by git

  • Commit changed from e25dd810b818d1543b66313d608ca33d93341b76 to a0ccf77b2d4af9e5a6fde8c01c4f0d1fdfff2bee

Branch pushed to git repo; I updated commit sha1. New commits:

2c9b36fRemove unused files
a0ccf77removing unused files: were used by the previously removed files.

comment:29 Changed 21 months ago by git

  • Commit changed from a0ccf77b2d4af9e5a6fde8c01c4f0d1fdfff2bee to 20d1f9e9fb6e4681fe45c41daf25b915788f8295

Branch pushed to git repo; I updated commit sha1. New commits:

b3c16a8Revert "removing unused files: were used by the previously removed files." Turns out interred is used somewhere else.
20d1f9eFix doctests in pbori.pyx

comment:30 Changed 21 months ago by git

  • Commit changed from 20d1f9e9fb6e4681fe45c41daf25b915788f8295 to bc1f9e24bdd9125fc48ed8de59232eb09e950e80

Branch pushed to git repo; I updated commit sha1. New commits:

bc1f9e2fix more doctests

comment:31 Changed 21 months ago by git

  • Commit changed from bc1f9e24bdd9125fc48ed8de59232eb09e950e80 to 64c57d2bc82f3920d8c91ca06669d5e4bb14df48

Branch pushed to git repo; I updated commit sha1. New commits:

b513981more doctests fixes
1cf36c7Remove VariableFactory from the lazy_import list for now.
64c57d2Last bunch of doctest fixes

comment:32 Changed 21 months ago by fbissey

  • Status changed from new to needs_review

This is ready for a first review. I am not expecting it to make it as is. But other people should comment now.

comment:33 Changed 21 months ago by git

  • Commit changed from 64c57d2bc82f3920d8c91ca06669d5e4bb14df48 to 878d941065c696fc38d9ec59d917a0ca1e3375da

Branch pushed to git repo; I updated commit sha1. New commits:

878d941Remove the sage_brial package since its meaningful content is merged.

comment:34 Changed 21 months ago by fbissey

Last commit to burn the bridges :)

comment:35 follow-up: Changed 21 months ago by mkoeppe

BooleanPolynomialRing_constructor vs. BooleanPolynomialRing looks a bit confusing

comment:36 in reply to: ↑ 35 ; follow-up: Changed 21 months ago by fbissey

Replying to mkoeppe:

BooleanPolynomialRing_constructor vs. BooleanPolynomialRing looks a bit confusing

Not my code, I am only the messenger. I certainly agree that this largely decade old code would need a good fleeing - apart from the trimming I already did on import. I assumed we could do it after merging it.

comment:37 Changed 21 months ago by mkoeppe

Sure, just trying to figure out what the high level interface is intended to be.

comment:38 Changed 21 months ago by mkoeppe

build/pkgs/sagelib/dependencies still has sage_brial

comment:39 Changed 21 months ago by fbissey

Right. I remember thinking about that last night. Forgot to remove it. Will do in a few hours if no one beats me to it.

comment:40 Changed 21 months ago by git

  • Commit changed from 878d941065c696fc38d9ec59d917a0ca1e3375da to 02a5af16c449d4839658aa5d7ac352dc2227f7d2

Branch pushed to git repo; I updated commit sha1. New commits:

02a5af1build/pkgs/sagelib/dependencies: Remove sage_brial

comment:41 Changed 21 months ago by mkoeppe

  • Authors set to François Bissey

comment:42 Changed 21 months ago by mkoeppe

  • Cc tscrim added

comment:43 in reply to: ↑ 36 Changed 21 months ago by tscrim

Replying to fbissey:

Replying to mkoeppe:

BooleanPolynomialRing_constructor vs. BooleanPolynomialRing looks a bit confusing

Not my code, I am only the messenger. I certainly agree that this largely decade old code would need a good fleeing - apart from the trimming I already did on import. I assumed we could do it after merging it.

Indeed, this is a known problem that needs some attention: #21892, #24748, possibly #27019, #23310, #23312, possibly #23311, #15223.

My idea is to scrap BooleanPolynomialRing_constructor and just have BooleanPolynomialRing be a UniqueRepresentation object, possibly just being a subclass of the monoid algebra parent.

comment:44 Changed 21 months ago by fbissey

My idea was to merge this in 9.2 and work on improving things in 9.3. However, if you know exactly what to do, we can work on this now.

comment:45 Changed 21 months ago by git

  • Commit changed from 02a5af16c449d4839658aa5d7ac352dc2227f7d2 to 0c5fe64317e93d2bf9f58d1418f5a73ecbb4968e

Branch pushed to git repo; I updated commit sha1. New commits:

754dea9Properly format tests in gbcore
0c5fe64Merge branch 'public/merge_sage_brial' of trac.sagemath.org:sage into brial-merging

comment:46 Changed 21 months ago by tscrim

I think that is a better plan. I am too stretched for time right now to work on this.

comment:47 Changed 21 months ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

I agree with this plan - it will also be a good basis for modularization work in 9.3

comment:48 Changed 21 months ago by fbissey

I am in the process of looking at adding some of the documentation from sage-brial to the sage documentation. At least to see how it looks like.

If you think that's not worth adding now, it is fine.

comment:49 Changed 21 months ago by git

  • Commit changed from 0c5fe64317e93d2bf9f58d1418f5a73ecbb4968e to 91fe5e1c34a3d0b0e4a2960119aa313ad51e687e
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

91fe5e1Fix doctest and docstring formatting

comment:50 Changed 21 months ago by fbissey

Just a touch up. One doctest was commented out so I decided to enable it. Building documentation revealed formatting issues in docstrings. All in the same file.

I built the doc with some added files from brial and we definitely should keep that for later work. Individual files need sensible title blocks for a start.

comment:51 Changed 21 months ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:52 Changed 21 months ago by vbraun

  • Branch changed from public/merge_sage_brial to 91fe5e1c34a3d0b0e4a2960119aa313ad51e687e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.