-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add min_length
and max_length
parameters to successors
/predecessors
#207
Comments
This seems like a pretty reasonable change that shouldn't blow up the API too much. Go for it! |
@Tagl This is a great idea, and seems very practical. I would suggest something like an optional @eliotwrobson What is our convention in the codebase for length-related variable/parameter names? So we use "length" or "len"? |
@caleb531 I think we usually use "length" (since "len" shadows the built-in function), but this might be inconsistent in some places. |
@eliotwrobson Yeah, now that I'm at my personal machine, I performed a quick search in the codebase. I see many uses of "length", e.g.:
So far, the only deviation I can find is Regardless, I think our established convention seems pretty evident from the above. And as for |
@Tagl have you been able to take a look at this? I actually have a use case where this would be very handy, so I'd like to include it in the version we release once the current package review is complete. |
Oh I thought I'd made the PR for this already. My bad! I'll find my implementation and get a PR set up soon. |
I think the
successors
/predecessors
functions are quite useful for enumerating words that are accepted by the DFA.Sometimes I want to limit the length of the word in an infinite language.
So far I have been limiting lengths by intersecting with
DFA.of_length
.It might be a good idea to add parameters for this, as it may be far more efficient to take apply that approach than to intersect with another DFA.
One additional benefit of this is it will guarantee that execution of the function ends, which is not true right now for infinite languages.
We might even want to require
max_length
or default to some reasonable upper bound based on the size of the alphabet.I'm planning on changing the implementation myself to support this in the near future, assuming it is beneficial with regards to performance.
@caleb531 @eliotwrobson do you have any thoughts on this?
The text was updated successfully, but these errors were encountered: