Closed Tagl closed 3 years ago
@Tagl Answering these in FIFO order 😉:
==
/ __eq__
).|
/ __or__
&
/ __and__
+
/ __add__
~
/ __invert__
reversed()
/ __reversed__
==
/ __eq__
Unfortunately, I don't think there's a practical operator to use for Kleene star, so I'd say just implement it as a method without any accompanying operator.
Feel free to submit me as many pull requests as you'd like as you implement operations. Again, adding tests are a requirement, but it will help you as much as it will help everyone else.
At any rate, thank you in advance for your willingness to contribute these operations!
Concatenation, reverse and Kleene star are best implemented by utilizing NFAs so I'm gonna wait with those for a while.
Additional operations which I have implemented:
<=
/ __le__
and <
/ __lt__
for proper subsets>=
/ __ge__
and >
/ __gt__
for proper supersets-
/ __sub__
^
/ __xor__
len
for cardinality.Should the naming follow the builtin set class with issubset
or should it be is_subset
?
@Tagl
Should the naming follow the builtin set class with issubset or should it be is_subset?
Definitely follow the builtin convention, i.e. issubset
Great work so far! Again, feel free to submit as many Pull Requests as you'd like! (with tests for your new operations, of course)
First pull request (#28) is in, let me know if anything needs changing.
I think it would be good to change the default behavior of minify to forget the original names and I would like to include that change with these operations. I think in most cases tracking states through their names is not very important.
Suppose you calculate the union of multiple state machines (for example in my case I'm taking unions of hundreds of DFAs) then the state names will become quite large and will slow down everything for probably no reason.
In the case that tracking them is desired then the methods can just be called with the retain_names
parameter set to True
@Tagl That all sounds fine to me! I already merged your PR, which seemed to contain all of the operations we've discussed here. I'm going to keep this issue open, though, since we still have NFA operations (like concatenation and Kleene star) to implement.
I am now laying the groundwork (documentation and all) for an automata v5 release, which your DFA operation contributions will be part of. I will make sure to give you credit for your welcome contribution in my release notes!
@Tagl Would it be possible for you to write a test case that covers the following code in _has_cycle
? That code is currently not covered by any existing tests. Please see my screenshots below.
The code in red never seemed to be hit my the tests.
The testcase that was supposed to cover this had 2 mistakes in it, I have fixed it and pushed.
@Tagl Thanks for that. I pulled down your code into my v5
branch. However, I did need to re-add your original test case in order to cover the if
in _has_cycle
. The code coverage is now at 100% again.
@Tagl I soon plan to publish the current operations you've implemented as part of a v4.1.0 release. Were you still planning to implement Kleene Star, Concatenation, and Reverse? Those appear to be the only operations which have not yet been implemented.
@caleb531 Yes I did intend to add those, I'll get around to it in the next few days.
@caleb531 I have submitted a PR (#38 ) which provides those operations.
Excellent, thank you, @Tagl! I will get to it within the next few days.
@Tagl I've reviewed your PR, and it looks very good! And I'll be happy to take care of updating the README with the documentation on these new operations.
Again, thank you so much! You have produced some really excellent work for this project. I plan to be releasing v5 very soon, at which point I will close this issue.
@Tagl And with that, I have just released Automata v5 with all the new operations you contributed! Thank you again!
I would like to have operations like these implemented within the library. The most common operations used are probably:
Another useful operation would be checking for equivalence of two DFAs. If an equivalence check is added should the eq method be overloaded with the equivalence check? I'd like to open a discussion on what additional operations could be added to the library. I have implementations for all the listed operations in mind and I have already written code for the first three so I'll gladly add these operations myself.