The point is exactly that the error message should be more meaningful. If a 
user doesn't expect an immutable object, it could be tuff to figure out the 
problem.

For example vectors give a more meaningful message, which I would just copy 
for matrices:

ValueError: vector is immutable; please change a copy instead (use copy())

As I'm not into graphs very much, I wanted to have some feedback before I 
just go along and open a ticket for this.

Am Donnerstag, 12. Dezember 2019 01:49:57 UTC+1 schrieb Travis Scrimshaw:
>
> Sorry, I think I misinterpreted the question. The solution I am proposing 
> is correct in some sense, but the better (or just another?) thing to do 
> would be to have the corresponding mutation methods of the immutable 
> backends raise a more appropriate error.
>
> Best,
> Travis
>
>
> On Thursday, December 12, 2019 at 10:47:26 AM UTC+10, Travis Scrimshaw 
> wrote:
>>
>> Hi Jonathan,
>>    You are trying to mutate an immutable graph (the default is 
>> inplace=True), so it is not surprising that it is throwing an error IMO. 
>> However, the error message and type should be improved; in particular 
>> reverse_edge() should raise an error if inplace=True and the graph is 
>> immutable.
>>
>> Best,
>> Travis
>>
>>
>> On Wednesday, December 11, 2019 at 9:58:05 PM UTC+10, Jonathan Kliem 
>> wrote:
>>>
>>> Dear all,
>>>
>>> when a graph is immutable, there are meaningless error messages that 
>>> make it hard for the user to guess what is going on (see below).
>>>
>>> When I create graphs like this, I know perfectly well what is going on. 
>>> However, when this is done in a cached method, it is hard for the user to 
>>> figure out the problem, isn't it?
>>>
>>> I'm currently working on #28828 <https://trac.sagemath.org/ticket/28828> 
>>> and returning an immutable graph seems to leave the user clueless about why 
>>> the code breaks. Of course one could just cache a private method and return 
>>> a copy or manually cache it, but that seems to be against the point of 
>>> @cached_method.
>>>
>>> Any suggestion for a good fix? Am I missing something?
>>>
>>> Thanks,
>>>
>>> Jonathan
>>>
>>> sage: G = Graph(immutable=True)
>>> sage: G.add_edge([0,1])
>>>
>>> ---------------------------------------------------------------------------
>>> NotImplementedError                       Traceback (most recent call 
>>> last)
>>> <ipython-input-20-2543f955ba93> in <module>()
>>> ----> 1 G.add_edge([Integer(0),Integer(1)])
>>>
>>> /home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/
>>> graphs/generic_graph.py in add_edge(self, u, v, label)
>>>   10816                     pass
>>>   10817 
>>> > 10818         self._backend.add_edge(u, v, label, self._directed)
>>>   10819 
>>>   10820     def add_edges(self, edges, loops=True):
>>>
>>> /home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/
>>> graphs/base/graph_backends.pyx in sage.graphs.base.graph_backends.
>>> GenericGraphBackend.add_edge (build/cythonized/sage/graphs/base/
>>> graph_backends.c:1694)()
>>>     100             NotImplementedError
>>>     101          """
>>> --> 102         raise NotImplementedError()
>>>     103 
>>>     104     def add_edges(self, edges, directed):
>>>
>>> NotImplementedError: 
>>>
>>>
>>> sage: G = DiGraph([[0,1]], immutable=True)
>>> sage: G.reverse_edges(G.edges())
>>>
>>> ---------------------------------------------------------------------------
>>> NotImplementedError                       Traceback (most recent call 
>>> last)
>>> <ipython-input-22-b7007f357194> in <module>()
>>> ----> 1 G.reverse_edges(G.edges())
>>>
>>> /home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/
>>> graphs/digraph.py in reverse_edges(self, edges, inplace, multiedges)
>>>    2074         tempG = self if inplace else copy(self)
>>>    2075         for e in edges:
>>> -> 2076             tempG.reverse_edge(e,inplace=True,multiedges=
>>> multiedges)
>>>    2077         if not inplace:
>>>    2078             return tempG
>>>
>>> /home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/
>>> graphs/digraph.py in reverse_edge(self, u, v, label, inplace, multiedges
>>> )
>>>    1968                                  "multiedges is True or False.")
>>>    1969         else:
>>> -> 1970             tempG.delete_edge(u, v, label)
>>>    1971             tempG.add_edge(v, u, label)
>>>    1972 
>>>
>>> /home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/
>>> graphs/generic_graph.py in delete_edge(self, u, v, label)
>>>   11130                     u, v = u
>>>   11131                     label = None
>>> > 11132         self._backend.del_edge(u, v, label, self._directed)
>>>   11133 
>>>   11134     def delete_edges(self, edges):
>>>
>>> /home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/
>>> graphs/base/graph_backends.pyx in sage.graphs.base.graph_backends.
>>> GenericGraphBackend.del_edge (build/cythonized/sage/graphs/base/
>>> graph_backends.c:2273)()
>>>     248             NotImplementedError
>>>     249         """
>>> --> 250         raise NotImplementedError()
>>>     251 
>>>     252     def del_vertex(self, v):
>>>
>>> NotImplementedError: 
>>>
>>>
>>>

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/9bbb60ee-ef2d-4855-af1c-0ddcac378005%40googlegroups.com.

Reply via email to