Opened 8 years ago
Closed 8 years ago
#16016 closed defect (fixed)
FiniteStateMachine.__and__ calls intersection and FiniteStateMachine.__or__ calls union.
Reported by: | skropf | Owned by: | |
---|---|---|---|
Priority: | trivial | Milestone: | sage-6.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, GitHub, GitLab) | 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 8 years ago by
comment:2 Changed 8 years ago by
comment:3 Changed 8 years ago by
- Status changed from new to needs_review
comment:4 follow-up: ↓ 5 Changed 8 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 ; follow-up: ↓ 10 Changed 8 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 8 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 8 years ago by
- 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
- Status changed from needs_review to positive_review
comment:9 follow-up: ↓ 12 Changed 8 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 8 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 8 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 8 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 8 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 8 years ago by
- Branch changed from u/skropf/fsm/and-intersection-or-union 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