Skip to content
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

Open
Tagl opened this issue Feb 7, 2024 · 6 comments · May be fixed by #224
Open

Add min_length and max_length parameters to successors/predecessors #207

Tagl opened this issue Feb 7, 2024 · 6 comments · May be fixed by #224
Assignees

Comments

@Tagl
Copy link
Contributor

Tagl commented Feb 7, 2024

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?

@eliotwrobson
Copy link
Collaborator

This seems like a pretty reasonable change that shouldn't blow up the API too much. Go for it!

@caleb531
Copy link
Owner

caleb531 commented Feb 8, 2024

@Tagl This is a great idea, and seems very practical. I would suggest something like an optional max_length parameter with a default value of None (meaning no limit). And min_length with a default value of 0.

@eliotwrobson What is our convention in the codebase for length-related variable/parameter names? So we use "length" or "len"?

@eliotwrobson
Copy link
Collaborator

@caleb531 I think we usually use "length" (since "len" shadows the built-in function), but this might be inconsistent in some places.

@caleb531
Copy link
Owner

caleb531 commented Feb 8, 2024

@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.:

  • DFA.minimum_word_length()
  • DFA.maximum_word_length()
  • DFA.words_of_length()
  • DFA.count_words_of_length()
  • DFA.of_length

So far, the only deviation I can find is _populate_count_cache_up_to_len, but that is an internal method (which perhaps we should rename for consistency).

Regardless, I think our established convention seems pretty evident from the above. And as for max_length vs. max_word_length, the DFA.of_length() method omits "word" in the parameter names to give min_length and max_length, so I think the latter are what we want to use here.

@eliotwrobson
Copy link
Collaborator

@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.

@Tagl
Copy link
Contributor Author

Tagl commented May 15, 2024

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.

@Tagl Tagl linked a pull request May 19, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants