Add a StreamWithState streamer #61
Conversation
src/raw/mod.rs
Outdated
| @@ -880,6 +957,7 @@ impl<'f, 'a, A: Automaton> Streamer<'a> for Stream<'f, A> { | |||
| let trans = state.node.transition(state.trans); | |||
| let out = state.out.cat(trans.out); | |||
| let next_state = self.aut.accept(&state.aut_state, trans.inp); | |||
| let aut_state = self.aut.accept(&state.aut_state, trans.inp); | |||
There was a problem hiding this comment.
I am asking for the state a second time because the Automaton State needs to be returned from this function and saved into the stack, so the other solution would be to ask a Cloneable Automaton State but it adds to many restriction to the entire code base (Clone and Clone constraints everywhere).
The solution would be:
- ask for a
CloneableStateonly for theStreamWithStateand in that case not be able to reuse theStreamWithStatenextmethod in theStreamnextmethod, - add
Cloneconstraints everywhere - the current one: ask two times for the state.
The other solution would be to return a &A::State, but sometimes the state is not stored into the Stream struct.
src/raw/mod.rs
Outdated
| return Some((&self.0.inp, out)); | ||
| } | ||
| } | ||
| None |
There was a problem hiding this comment.
I don't like code duplication, the only thing different with the StreamWithState is the clone of the aut_state.
|
There is a lot of new code here. Could you please describe what problem you're trying to solve here? |
|
Here I updated the original PR description. I removed many useless methods and types implementations. All these commits needs to be squashed. EDIT: Ok so the tests are broken, I will find a way to keep one implementation of |
|
I found a great solution 🎉 I updated the original PR description to reflect the last changes. Functions documentations and other methods are missing (e.g. |
|
This issue will never be merged as it adds to many complexity to the library and this is not useful for most of the users. |
| vs.push((k.to_vec(), v.value())); | ||
| } | ||
| vs | ||
| } |
There was a problem hiding this comment.
Why were all of these methods removed?
There was a problem hiding this comment.
Those weren't removed, it is because git doesn't understand that it is a new struct named StreamWithState that I created and I didn't implement those methods on it to keep the code easier to follow.
|
I am going to look into merging this PR, but will probably re-implement it myself. There are some changes here that I don't quite understand yet. |
|
Notice that even if you would have wanted to merge it, it wouldn't have been possible as I deleted the repo and branch. A great idea of mine, again... |
|
@Kerollmops No worries! :) And thanks for responding. |
This is apparently useful in some cases when access to the underlying automaton's state can produce useful information about a match. Regretably, the implementation requires the states to satisfy `Clone`, or else we would otherwise need to execute the state transition twice. In practice, states are usually `Copy` and quite small. Closes #60, Closes #61
|
An implementation of this is now in my rollup branch: b1e7441 Feedback is welcome, but I think I pretty much followed the ideas in this PR. |
This is apparently useful in some cases when access to the underlying automaton's state can produce useful information about a match. Regretably, the implementation requires the states to satisfy `Clone`, or else we would otherwise need to execute the state transition twice. In practice, states are usually `Copy` and quite small. Closes #60, Closes #61
This is apparently useful in some cases when access to the underlying automaton's state can produce useful information about a match. Regretably, the implementation requires the states to satisfy `Clone`, or else we would otherwise need to execute the state transition twice. In practice, states are usually `Copy` and quite small. Closes #60, Closes #61
This is apparently useful in some cases when access to the underlying automaton's state can produce useful information about a match. Regretably, the implementation requires the states to satisfy `Clone`, or else we would otherwise need to execute the state transition twice. In practice, states are usually `Copy` and quite small. Closes #60, Closes #61
This is apparently useful in some cases when access to the underlying automaton's state can produce useful information about a match. Regretably, the implementation requires the states to satisfy `Clone`, or else we would otherwise need to execute the state transition twice. In practice, states are usually `Copy` and quite small. Closes #60, Closes #61
This is apparently useful in some cases when access to the underlying automaton's state can produce useful information about a match. Regretably, the implementation requires the states to satisfy `Clone`, or else we would otherwise need to execute the state transition twice. In practice, states are usually `Copy` and quite small. Closes #60, Closes #61
This is an attempt to be able to retrieve the
AutomatonStatefrom a streamer, this streamer will be calledStreamWithState.Currently the
map::Streamstruct implement theStreamertrait with an associatedStreamer::Itemthat is(&[u8], u64)in other term the matching string associated with the value.The problem is that it is not possible to retrieve the state of the automaton with the current
map::Streamtype. It could be useful when themap::Streamis used with a levenshtein automaton, the state will permit to know the distance of the matching string returned.So I propose to add a new struct named
StreamWithStatethat implement theStreamertrait and its associated typeStreamer::Itemis(&[u8], u64, A::State)whereA: Automaton. This way the user will be able to retrieve the state of theAutomatonand therefore the levenshtein distance of the matching string returned (by answering the automaton).The
StreamWithStateis accessible via thewith_stateself consuming method onmap::StreamBuilder.All of this resolves #60.
What I have done here is probably a little bit confused due to the way github shows the diffs.
For me,
map::StreamWithStateis a superset ofraw::StreamIt should be possible to implementmap::Streamusing the other and just ignoring theAutomaton::State, so I created a specialStreamWithState::nextmethod that accept a function which will "transform" theA::State:map::Streamwhich does not need the state, ignores it by using a function that return().map::StreamWithStateclones it by using theClone::clonefunction.