Opened 8 years ago

Closed 8 years ago

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

Reported by: Owned by: skropf trivial sage-6.2 combinatorics finite_state_machine dkrenn, cheuberg, slabbe Sara Kropf Nathann Cohen N/A 3c34436 3c34436d2329c13d6cbcf198e15642d72b51a8f2

### Description

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.

### comment:1 Changed 8 years ago by skropf

• Authors set to Sara Kropf

### comment:2 Changed 8 years ago by ncohen

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

### comment:3 Changed 8 years ago by skropf

• Status changed from new to needs_review

### comment:4 follow-up: ↓ 5 Changed 8 years ago by ncohen

• Status changed from needs_review to needs_info

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

Nathann

### comment:5 in reply to: ↑ 4 ; follow-up: ↓ 10 Changed 8 years ago by 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.

### comment:6 Changed 8 years ago by git

• Commit changed from ee5c29553460b3dac2db0c092433f9a2297c3cff to 3c34436d2329c13d6cbcf198e15642d72b51a8f2

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

 ​3c34436 `FiniteStateMachine.__add__ is the same as __or__`

### comment:7 Changed 8 years ago by skropf

• Status changed from needs_info to needs_review

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

### comment:8 Changed 8 years ago by ncohen

• Status changed from needs_review to positive_review

Oooooooooookayyyyyyyyyyyyyyyyyyyyyyyy !!!

Nathann

### comment:9 follow-up: ↓ 12 Changed 8 years ago by ncohen

• Status changed from positive_review to needs_work

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

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

### comment:10 in reply to: ↑ 5 Changed 8 years ago by slabbe

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

### comment:11 Changed 8 years ago by ncohen

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

Nathann

### comment:12 in reply to: ↑ 9 Changed 8 years ago by skropf

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?

### comment:13 Changed 8 years ago by ncohen

• Reviewers set to Nathann Cohen
• Status changed from needs_work to positive_review

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

Nathann

### comment:14 Changed 8 years ago by vbraun

• Branch changed from u/skropf/fsm/and-intersection-or-union to 3c34436d2329c13d6cbcf198e15642d72b51a8f2
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.