Immutable graphs must not be relabeled

[ncohen]

Now

sage: Graph(graphs.PetersenGraph(), immutable=True).relabel({})
...
ValueError: Thou shalt not relabel an immutable graph

Before, the following happened

sage: Graph(graphs.PetersenGraph(), data_structure="static_sparse").relabel({})
...
AttributeError: 'StaticSparseBackend' object has no attribute 'vertex_ints'

This is actually another bug, as there should be a vertex_ints variable in static sparse graphs. But that's another problem to be addressed in another ticket :-P

Nathann

[git]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

​0f57805	trac #15622: Immutable graphs must not be relabeled
​c441178	trac #15619: bug in the former definition; exception to avoid it in the future
​0229348	trac #15619: Pickling of immutable graphs
​7860f39	trac #15603: "immutable=True" for Graph/Digraph __init__ and copy()
​2525d22	Trac 15278: Fix syntax error in doc test
​fcf9e30	Trac 15278: Only graphs that use the static backend are identical with their copy
​8fc9c94	Merge branch 'develop' into ticket/15278
​51d6328	Trac 15278: Error messages explain how to create (im)mutable graph copy
​2fc8a77	Make digraphs using the static backend immutable and hashable
​07bad46	Merge branch 'u/ncohen/15491' of git://trac.sagemath.org/sage into ticket/15278

[SimonKing]

The code looks good. Perhaps the error message should hint at how to create a mutable copy?

[ncohen]

OKayyyyyyyy.. What about this ?

Nathann

[git]

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

​529f785

trac #15622: More informative exception message

[SimonKing]

Oops:

sage -t --long src/sage/graphs/base/static_sparse_backend.pyx
**********************************************************************
File "src/sage/graphs/base/static_sparse_backend.pyx", line 422, in sage.graphs.base.static_sparse_backend.StaticSparseBackend.relabel
Failed example:
    g.relabel([],True)
Expected:
    Traceback (most recent call last):
    ...
    ValueError: Thou shalt not remove a vertex from an immutable graph
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 480, in _run
        self.execute(example, compiled, test.globs)
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 839, in execute
        exec compiled in globs
      File "<doctest sage.graphs.base.static_sparse_backend.StaticSparseBackend.relabel[2]>", line 1, in <module>
        g.relabel([],True)
    AttributeError: 'sage.graphs.base.static_sparse_backend.StaticSpars' object has no attribute 'relabel'

That is the only error, and I'll probably fix it in a review commit.

[SimonKing]

WTF??????

When I do the obvious and change StaticSparseCGraph into StaticSparseBackend, I get

Failed example:
    g.relabel([],True)
Expected:
    Traceback (most recent call last):
    ...
    ValueError: Thou shalt not remove a vertex from an immutable graph
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 480, in _run
        self.execute(example, compiled, test.globs)
      File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 839, in execute
        exec compiled in globs
      File "<doctest sage.graphs.base.static_sparse_backend.StaticSparseBackend.relabel[2]>", line 1, in <module>
        g.relabel([],True)
      File "static_sparse_backend.pyx", line 428, in sage.graphs.base.static_sparse_backend.StaticSparseBackend.relabel (sage/graphs/base/static_sparse_backend.c:4740)
    ValueError: Thou shalt not relabel an immutable graph

Where does the blankline come from? And why is there no blankline when doing the test interactively?

[SimonKing]

Ahaaa! You had a wrong class and a wrong error message in your test ("remove a vertex" versus "relabel").

[ncohen]

I don't get what the "obvious change from StaticSparseCGraph into StaticSparseBackend" could be O_o

Nathann

[SimonKing]

Replying to ncohen:

I don't get what the "obvious change from StaticSparseCGraph into StaticSparseBackend" could be O_o

src/sage/graphs/base/static_sparse_backend.pyx

@@ -417 +417 @@
 
         TEST::
 
-            sage: from sage.graphs.base.static_sparse_backend import StaticSparseCGraph
-            sage: g = StaticSparseCGraph(graphs.PetersenGraph())
+            sage: from sage.graphs.base.static_sparse_backend import StaticSparseBackend
+            sage: g = StaticSparseBackend(graphs.PetersenGraph())
             sage: g.relabel([],True)
             Traceback (most recent call last):
             ...
-            ValueError: Thou shalt not remove a vertex from an immutable graph
+            ValueError: Thou shalt not relabel an immutable graph
 
         """
         raise ValueError("Thou shalt not relabel an immutable graph")

StaticSparceCGraph simply is the wrong class.

[SimonKing]

For the record: It already takes 5 minutes to push my review commit.

[ncohen]

Oh. Ahem. Right O_O;;;;;;

Sorryyyyyyyyyyyyyyyyyyyyyyyyyyy O_O;;;;;

Nathann

Review commit : I often Ctrl+C it before it ends. Trac sends the mails almost instantaneously, the trac ticket is updated at the same time. I just have no idea what it does afterwards.

[SimonKing]

It seems that once more I have to manually upload the commit.

[ncohen]

I think it's because you should set the branch name *before* pushing the commit. Otherwise nothing is triggered on the trac server's side.

Nathann

[ncohen]

Okay. We are now in the Volker-Feared situation of a ticket which has a positive review, with a dependency which is still in needs_review :-P

Thanks for this commit !

Nathann

[ncohen]

By the way, isn't the trac server 99999x times faster than yesterday ? Andrew said he would do something about it and it looks like he did O_O

Nathann

[vbraun]

Various doctests fail (see also patchbot)

[ncohen]

Gloops. Right.

[ncohen]

That's the only way I found to make this work before #15623 is merged T_T

Nathann

[ncohen]

Okay, it looks like the automatic message does not work, but the branch is updated !

Nathann

[SimonKing]

Replying to vbraun:

Various doctests fail (see also patchbot)

WTF? When I reviewed it, things worked. Or did I accidentally merge other tickets that aren't dependencies for this?

[SimonKing]

Hooray. After pulling from the ticket branch and merging with the current develop branch, make ptest results in

All tests passed!
----------------------------------------------------------------------
Total time for all tests: 3791.7 seconds
    cpu time: 6340.1 seconds
    cumulative wall time: 7304.6 seconds

Nathann's last commit looks like the right thing to do. Hence, I hope this time my positive review will persist...