daily WTF?

[William Stein]

At Sage Days a new developer we're onboarding decided to look at some
code in Sage and ran into this:

if sum([G.is_directed(), H.is_directed()]) == 1:
raise ValueError("One graph can not be directed while the
other is not.")

This is line 551 of src/sage/graphs/generic_graph_pyx.pyx:

https://github.com/sagemath/sage/blame/master/src/sage/graphs/generic_graph_pyx.pyx#L551

It seems to be equivalent to

if G.is_directed() != H.is_directed():
raise ValueError("One graph can not be directed while the
other is not.")

Frankly, I'm really embarrassed about this code. What do you guys think?

-- William


--
William (http://wstein.org)

[Volker Braun]

The many ways of implementing xor...

Incidentally, if there is some global switch to return certificates instead of booleans then the sum version is the correct one ;-)

[cir...@gmail.com]

The sum version works of course, but I'd favor the other code because it is so much clearer. It's the difference between having to figure out what is intended and being able to see at a glance the meaning of the code.

[leif]

Volker Braun wrote:
> The many ways of implementing xor...
>
> Incidentally, if there is some global switch to return certificates
> instead of booleans then the sum version is the correct one ;-)

Besides the perhaps clumsy syntax in Python, it's IMHO ok. Just imagine
we had a list of more than two graphs; actually 'sum(l)==1' should be
'sum(l) not in (0, len(l))'.

Of course there are nicer versions with map(), filter(), any() or all()
etc. (sum() is of course also just an application of reduce(), just
like any() and all() are, but the latter do not necessarily have to
evaluate each list element, so can be faster in the average case.)


-leif

P.S.: Wouldn't it make more sense to raise a TypeError? After all,
directed and undirected graphs are different structures.

[Dima Pasechnik]

On Saturday, August 20, 2016 at 2:09:34 PM UTC+1, leif wrote:

Volker Braun wrote:
> The many ways of implementing xor...
>
> Incidentally, if there is some global switch to return certificates
> instead of booleans then the sum version is the correct one ;-)

Besides the perhaps clumsy syntax in Python, it's IMHO ok.  Just imagine
we had a list of more than two graphs;  actually 'sum(l)==1' should be
'sum(l) not in (0, len(l))'.

Of course there are nicer versions with map(), filter(), any() or all()
etc.  (sum() is of course also just an application of reduce(), just
like any() and all() are, but the latter do not necessarily have to
evaluate each list element, so can be faster in the average case.)


-leif

P.S.:  Wouldn't it make more sense to raise a TypeError?  After all,
directed and undirected graphs are different structures.

they are different, but there are canonical ways to convert one into the other.

directed => undirected -- forget the direction;

undirected => directed -- replace every edge {a,b} by a pair of arcs (a,b),(b,a)

So it's an implementation detail, one could instead agree on a conversion done

automatically.

[leif]

Not really. Undirected graphs are a special case of directed graphs,
and even more so with edge labeling.

I don't even know whether Sage considers the empty graph (and graphs
without edges) directed, undirected, both, or none of that...

(I *guess* that mainly depends on how you construct them, so again a
matter of types or classes.)


-leif

[leif]

P.S.: So you think for now, we should raise NotImplementedError instead?


-leif

[Robert Dodier]

On 2016-08-20, Volker Braun <vbrau...@gmail.com> wrote:

> The many ways of implementing xor...
>
> Incidentally, if there is some global switch to return certificates instead
> of booleans then the sum version is the correct one ;-)

I don't understand this point, but it sounds interesting. Can you
explain?

best

Robert Dodier