search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.37k stars 469 forks source link

FiniteStateMachine.__and__ calls intersection and FiniteStateMachine.__or__ calls union. #16016

Closed c50b3d32-6cb1-4b90-a060-6a332e54ef6a closed 10 years ago

c50b3d32-6cb1-4b90-a060-6a332e54ef6a commented 10 years ago

Nevertheless, intersection and union are still not implemented. Thus, this only changes the names of the functions. Previously __mul__ called intersection and __add__ called union.

This is an answer to one of the comments in #15078 comment:32.

CC: @dkrenn @cheuberg @seblabbe

Component: combinatorics

Author: Sara Kropf

Branch/Commit: 3c34436

Reviewer: Nathann Cohen

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

c50b3d32-6cb1-4b90-a060-6a332e54ef6a commented 10 years ago

Author: Sara Kropf

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 10 years ago
comment:2

I believe that | and + should be aliases for union. That's how it works for sets already

sage: Set([1,2,3])+Set([3,4,6])
{1, 2, 3, 4, 6}
sage: Set([1,2,3])|Set([3,4,6])
{1, 2, 3, 4, 6}

And for graphs only + is defined

sage: graphs.PetersenGraph() + graphs.ChvatalGraph()
Petersen graph disjoint_union Chvatal graph: Graph on 22 vertices
sage: graphs.PetersenGraph() | graphs.ChvatalGraph()
...
TypeError: unsupported operand type(s) for |: 'Graph' and 'Graph'

If you agree with that you can add a __or__ = __add__ after the function's definition :-)

Nathann

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 10 years ago
comment:4

Well, could you answer yesterday's question ? What do you think ?

Nathann

c50b3d32-6cb1-4b90-a060-6a332e54ef6a commented 10 years ago
comment:5

For me, that sounds good. But I would like to wait for possible other opinions on this topic before I change it, since originally it was suggested to use only __or__ (instead of __add__).

I will wait until Monday.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

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

3c34436FiniteStateMachine.__add__ is the same as __or__
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 10 years ago

Changed commit from ee5c295 to 3c34436

c50b3d32-6cb1-4b90-a060-6a332e54ef6a commented 10 years ago
comment:7

Now __add__ does the same as __or__, as suggested above.

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 10 years ago
comment:8

Oooooooooookayyyyyyyyyyyyyyyyyyyyyyyy !!!

http://grooveshark.com/s/Let+s+Bang/3S4pl4?src=5

Nathann

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 10 years ago
comment:9

OOops, nononono. It does not pass tests :-)

sage -t --long finite_state_machine.py  # 2 doctests failed
seblabbe commented 10 years ago
comment:10

Replying to @sagetrac-skropf:

For me, that sounds good. But I would like to wait for possible other opinions on this topic before I change it, since originally it was suggested to use only __or__ (instead of __add__).

I will wait until Monday.

I am Ok for __add__ having the same behavior as __or__.

But, do we agree that __mul__ should not link to __and__ ? To me, if A and B are two automata, A * B means the concatenation of them, not the intersection.

Sébastien

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 10 years ago
comment:11

Yepyep, in the current patch we only have an alias from add to or.

Nathann

c50b3d32-6cb1-4b90-a060-6a332e54ef6a commented 10 years ago
comment:12

Replying to @nathanncohen:

OOops, nononono. It does not pass tests :-)

sage -t --long finite_state_machine.py  # 2 doctests failed

I don't know what you mean. For me, all doctests pass.

Can you check it again, please, and tell me your error messages?

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 10 years ago
comment:13

My mistake ! I must have forgotten to recompile or something. Sorry for that, good to go :-)

Nathann

6bdad4c1-1e26-4f2f-a442-a01a2292c181 commented 10 years ago

Reviewer: Nathann Cohen

vbraun commented 10 years ago

Changed branch from u/skropf/fsm/and-intersection-or-union to 3c34436