Opened 5 years ago
Closed 5 years ago
#16016 closed defect (fixed)
FiniteStateMachine.__and__ calls intersection and FiniteStateMachine.__or__ calls union.
Reported by:  skropf  Owned by:  

Priority:  trivial  Milestone:  sage6.2 
Component:  combinatorics  Keywords:  finite_state_machine 
Cc:  dkrenn, cheuberg, slabbe  Merged in:  
Authors:  Sara Kropf  Reviewers:  Nathann Cohen 
Report Upstream:  N/A  Work issues:  
Branch:  3c34436 (Commits)  Commit:  3c34436d2329c13d6cbcf198e15642d72b51a8f2 
Dependencies:  Stopgaps: 
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.
This is an answer to one of the comments in ticket:15078#comment:32.
Change History (14)
comment:1 Changed 5 years ago by
comment:2 Changed 5 years ago by
comment:3 Changed 5 years ago by
 Status changed from new to needs_review
comment:4 followup: ↓ 5 Changed 5 years ago by
 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 ; followup: ↓ 10 Changed 5 years ago by
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 5 years ago by
 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 5 years ago by
 Status changed from needs_info to needs_review
Now __add__
does the same as __or__
, as suggested above.
comment:8 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:9 followup: ↓ 12 Changed 5 years ago by
 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 5 years ago by
Replying to 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
comment:11 Changed 5 years ago by
Yepyep, in the current patch we only have an alias from add to or.
Nathann
comment:12 in reply to: ↑ 9 Changed 5 years ago by
Replying to ncohen:
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 5 years ago by
 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 5 years ago by
 Branch changed from u/skropf/fsm/andintersectionorunion to 3c34436d2329c13d6cbcf198e15642d72b51a8f2
 Resolution set to fixed
 Status changed from positive_review to closed
I believe that  and + should be aliases for union. That's how it works for sets already
And for graphs only + is defined
If you agree with that you can add a
__or__ = __add__
after the function's definition:)
Nathann