Opened 3 years ago
Closed 2 years ago
#24786 closed defect (fixed)
py3: various Python 3 bugs in the brial package
Reported by:  embray  Owned by:  

Priority:  major  Milestone:  sage8.4 
Component:  python3  Keywords:  brial 
Cc:  jdemeyer  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Erik Bray 
Report Upstream:  Fixed upstream, in a later stable release.  Work issues:  
Branch:  22fc642 (Commits)  Commit:  22fc642cf90d084584361bef53828993f4e72140 
Dependencies:  Stopgaps: 
Description (last modified by )
This error occurs in several places; e.g.
sage t src/sage/rings/polynomial/multi_polynomial_sequence.py ********************************************************************** File "src/sage/rings/polynomial/multi_polynomial_sequence.py", line 106, in sage.rings.polynomial.multi_polynomial_sequence Failed example: C[0].groebner_basis() Exception raised: Traceback (most recent call last): File "/home/embray/src/sagemath/sagepython3/local/lib/python3.6/sitepackages/sage/doctest/forker.py", line 556, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/embray/src/sagemath/sagepython3/local/lib/python3.6/sitepackages/sage/doctest/forker.py", line 966, in compile_and_execute exec(compiled, globs) File "<doctest sage.rings.polynomial.multi_polynomial_sequence[7]>", line 1, in <module> C[Integer(0)].groebner_basis() File "/home/embray/src/sagemath/sagepython3/local/lib/python3.6/sitepackages/sage/rings/polynomial/multi_polynomial_sequence.py", line 530, in groebner_basis return self.ideal().groebner_basis(*args, **kwargs) File "sage/rings/polynomial/pbori.pyx", line 5070, in sage.rings.polynomial.pbori.BooleanPolynomialIdeal.groebner_basis (build/cythonized/sage/rings/polynomial/pbori.cpp:42826) gb = self._groebner_basis(**kwds) File "sage/rings/polynomial/pbori.pyx", line 5100, in sage.rings.polynomial.pbori.BooleanPolynomialIdeal._groebner_basis (build/cythonized/sage/rings/polynomial/pbori.cpp:43222) from brial.gbcore import groebner_basis File "/home/embray/src/sagemath/sagepython3/local/lib/python3.6/sitepackages/brial/__init__.py", line 31, in <module> from .PyPolyBoRi import * File "/home/embray/src/sagemath/sagepython3/local/lib/python3.6/sitepackages/brial/PyPolyBoRi.py", line 74, in <module> OrderCode.__dict__ = order_dict AttributeError: attribute '__dict__' of 'type' objects is not writable
There are also several other minor Python 3 bugs in brial
.
Upstream PR: https://github.com/BRiAl/BRiAl/pull/33
New release of BRial: https://github.com/BRiAl/BRiAl/releases/download/1.2.4/brial1.2.4.tar.bz2
Change History (32)
comment:1 Changed 3 years ago by
comment:2 Changed 3 years ago by
That's fairI've been thinking about that myself. I almost feel like we need some kind of status board for Python 3. I'll see what I can come up with.
As for the "recent flurry of activity" almost all of this is work I did some time ago but never got around to organizing into comprehensible discrete tickets. Most of it is miscellaneous and not directly tied to each other (I will usually make clear when multiple tickets are related to each other).
comment:3 Changed 3 years ago by
Here are the lines in https://github.com/BRiAl/BRiAl/blob/master/sagebrial/brial/PyPolyBoRi.py
class OrderCode: pass OrderCode.__dict__ = order_dict OrderCode.__module__ = __name__
comment:4 Changed 3 years ago by
There is a newer version of brial but if there is still bugs I am happy to merge PR.
comment:5 Changed 3 years ago by
 Description modified (diff)
 Milestone changed from sage8.2 to sage8.3
 Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.
 Status changed from new to needs_review
 Summary changed from py3: error in brial.PyPolyBoRi to py3: various Python 3 bugs in the brial package
comment:6 Changed 2 years ago by
 Status changed from needs_review to needs_info
This says "needs review". What should be reviewed here?
comment:7 Changed 2 years ago by
 Report Upstream changed from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.
The PR to brial did need review, but this is done now.
What we need to do now is either add this patch to Sage, or release a new version of brial and update Sage to use it (preferably the latter). But let's wait to see if I can identify any more Python 3 bugs in brial (I don't think there are but it's feasible).
comment:8 Changed 2 years ago by
I can release a new brial when you think it is ready. The main reason I have that power on the brial repo is to be able to do releases for sage in a timely fashion. Although I also did quite a bit of clean up with the help of other sageondistro maintainers in the last few months.
comment:9 followup: ↓ 10 Changed 2 years ago by
@fbissey
Please make a release of brial. This will be helpful for python3 progress.
comment:10 in reply to: ↑ 9 Changed 2 years ago by
Replying to chapoton:
@fbissey
Please make a release of brial. This will be helpful for python3 progress.
I may have time tomorrow but that will be a squeeze. Better count on it on Tuesday (NZ time).
comment:11 Changed 2 years ago by
New release out for you at https://github.com/BRiAl/BRiAl/releases/download/1.2.4/brial1.2.4.tar.bz2
Do check that it has all the changes you need.
comment:12 Changed 2 years ago by
 Report Upstream changed from Fixed upstream, but not in a stable release. to Fixed upstream, in a later stable release.
 Status changed from needs_info to needs_work
now we need to update brial here
comment:13 Changed 2 years ago by
Are you saying I should make a branch to upgrade Sage's brial to this version?
comment:14 Changed 2 years ago by
Hello Erik, and welcome back.
Well, I think we should upgrade brial, indeed. Either here or in another ticket.
Maybe Pynac upgrade is more important than this one for being able to make the doc.
comment:15 Changed 2 years ago by
Thanks!
And yes, IIRC the pynac updates are especially critical for Python 3 support (otherwise there are segfaults, etc.)
comment:16 Changed 2 years ago by
I can put the brial update in this ticket.
comment:17 Changed 2 years ago by
@embray : yes, please, let us do the brial update here
comment:18 Changed 2 years ago by
 Description modified (diff)
comment:19 Changed 2 years ago by
 Branch set to public/brial_1.2.4
 Commit set to fcba0bf5ba89b70c35553c215493ac4d1a94964b
New commits:
fcba0bf  update brial to 1.2.4

comment:20 Changed 2 years ago by
patches do not apply.. And I am not able to do the required refreshing of patches, sorry
comment:21 Changed 2 years ago by
 Commit changed from fcba0bf5ba89b70c35553c215493ac4d1a94964b to 22fc642cf90d084584361bef53828993f4e72140
Branch pushed to git repo; I updated commit sha1. New commits:
22fc642  removing brial patch

comment:22 Changed 2 years ago by
 Cc jdemeyer added
 Status changed from needs_work to needs_review
turns out the patch was no longer needed. So this should be good now
comment:24 followup: ↓ 25 Changed 2 years ago by
 Status changed from needs_work to needs_review
well, seems to work after sage f brial
so setting back to needs review.
comment:25 in reply to: ↑ 24 Changed 2 years ago by
Replying to chapoton:
well, seems to work after
sage f brial
so setting back to needs review.
I was a bit surprised and was going to ask for more details.
comment:26 followup: ↓ 27 Changed 2 years ago by
LGTM. Jeroen should probably comment since he wrote the patch.
comment:27 in reply to: ↑ 26 Changed 2 years ago by
Replying to embray:
LGTM. Jeroen should probably comment since he wrote the patch.
Why? The patch was from a pull request he made to BRiAl and which I have merged. So the patch is included in this version of BRiAl. https://github.com/BRiAl/BRiAl/pull/15
comment:28 Changed 2 years ago by
 Reviewers set to Erik Bray
 Status changed from needs_review to positive_review
Ok. That wasn't immediately obvious. It looks like my PR is included as well.
comment:29 Changed 2 years ago by
Author name.
comment:30 Changed 2 years ago by
comment:31 Changed 2 years ago by
 Milestone changed from sage8.3 to sage8.4
I believe this issue can reasonably be addressed for Sage 8.4.
comment:32 Changed 2 years ago by
 Branch changed from public/brial_1.2.4 to 22fc642cf90d084584361bef53828993f4e72140
 Resolution set to fixed
 Status changed from positive_review to closed
Erik, could you please post on sagedevel some kind of report of your progress on python3 ? In particular, I would like to know how far we still are
(0) from sage starting (almost there, I think it may just need #24728),
(1) from being able to build the doc (#24774 and then ?),
(2) from having a functional doctest framework (#24343 and what else ?).
And what are the involved tickets for each of these objectives ? I must say that I am a bit lost in your recent flurry of activity. I would like to put some priority between all these tickets.