py3: various Python 3 bugs in the brial package

[embray]

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/sage-python3/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 556, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/embray/src/sagemath/sage-python3/local/lib/python3.6/site-packages/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/sage-python3/local/lib/python3.6/site-packages/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/sage-python3/local/lib/python3.6/site-packages/brial/__init__.py", line 31, in <module>
        from .PyPolyBoRi import *
      File "/home/embray/src/sagemath/sage-python3/local/lib/python3.6/site-packages/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/brial-1.2.4.tar.bz2

Upstream: Fixed upstream, in a later stable release.

CC: @jdemeyer

Component: python3

Keywords: brial

Author: Frédéric Chapoton

Branch/Commit: 22fc642

Reviewer: Erik Bray

Issue created by migration from https://trac.sagemath.org/ticket/24786

[fchapoton]

comment:1

Erik, could you please post on sage-devel 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.

[embray]

comment:2

That's fair--I'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).

[fchapoton]

comment:3

Here are the lines in https://github.com/BRiAl/BRiAl/blob/master/sage-brial/brial/PyPolyBoRi.py

class OrderCode:
    pass

OrderCode.__dict__ = order_dict
OrderCode.__module__ = __name__

[kiwifb]

comment:4

There is a newer version of brial but if there is still bugs I am happy to merge PR.

[embray]

Changed upstream from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.

[embray]

Description changed:

--- 
+++ 
@@ -27,4 +27,6 @@
     AttributeError: attribute '__dict__' of 'type' objects is not writable

-I haven't looked closely at this code yet, but presumably in Python 2 OrderCode is an old-style class which allowed assignment to their __dict__ attributes, whereas new-style classes do not allow this. Whatever it's trying to accomplish by assigning to __dict__ can surely be done another way. +There are also several other minor Python 3 bugs in brial. + +Upstream PR: https://github.com/BRiAl/BRiAl/pull/33

[saraedum]

comment:6

This says "needs review". What should be reviewed here?

[embray]

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.

[embray]

comment:7

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

[kiwifb]

comment:8

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 sage-on-distro maintainers in the last few months.

[fchapoton]

comment:9

@kiwifb

Please make a release of brial. This will be helpful for python3 progress.

[kiwifb]

comment:10

Replying to @fchapoton:

@kiwifb

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

[kiwifb]

comment:11

New release out for you at https://github.com/BRiAl/BRiAl/releases/download/1.2.4/brial-1.2.4.tar.bz2

Do check that it has all the changes you need.

[fchapoton]

comment:12

now we need to update brial here

[fchapoton]

Changed upstream from Fixed upstream, but not in a stable release. to Fixed upstream, in a later stable release.

[embray]

comment:13

Are you saying I should make a branch to upgrade Sage's brial to this version?

[fchapoton]

comment:14

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.

[embray]

comment:15

Thanks!

And yes, IIRC the pynac updates are especially critical for Python 3 support (otherwise there are segfaults, etc.)

[embray]

comment:16

I can put the brial update in this ticket.

[fchapoton]

comment:17

@embray : yes, please, let us do the brial update here

[fchapoton]

Description changed:

--- 
+++ 
@@ -30,3 +30,6 @@
 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/brial-1.2.4.tar.bz2

[fchapoton]

Branch: public/brial_1.2.4

[fchapoton]

New commits:

fcba0bf

update brial to 1.2.4

[fchapoton]

Commit: fcba0bf

[fchapoton]

comment:20

patches do not apply.. And I am not able to do the required refreshing of patches, sorry

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from fcba0bf to 22fc642

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

22fc642

removing brial patch

[fchapoton]

comment:22

turns out the patch was no longer needed. So this should be good now

[fchapoton]

comment:23

sage crash..

[fchapoton]

comment:24

well, seems to work after sage -f brial

so setting back to needs review.

[kiwifb]

comment:25

Replying to @fchapoton:

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.

[embray]

comment:26

LGTM. Jeroen should probably comment since he wrote the patch.

[kiwifb]

comment:27

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

[embray]

Reviewer: Erik Bray

[embray]

comment:28

Ok. That wasn't immediately obvious. It looks like my PR is included as well.

[tscrim]

comment:29

Author name.

[fchapoton]

Author: Frédéric Chapoton

[embray]

comment:31

I believe this issue can reasonably be addressed for Sage 8.4.

[vbraun]

Changed branch from public/brial_1.2.4 to 22fc642