Skip to content

Simplified binary operations for DFAs #139

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

Merged
merged 9 commits into from
Apr 11, 2023

Conversation

EduardoGoulart1
Copy link
Contributor

This is the PR. Hope it is good to go now

- Extract generic BFS code
@coveralls
Copy link

coveralls commented Apr 5, 2023

Coverage Status

Coverage: 100.0%. Remained the same when pulling da0c141 on EduardoGoulart1:simplify-binary-operations into e8605ad on caleb531:develop.

@EduardoGoulart1
Copy link
Contributor Author

EduardoGoulart1 commented Apr 5, 2023

See #133 for past discussion

@EduardoGoulart1 EduardoGoulart1 force-pushed the simplify-binary-operations branch from 8dbd1b2 to 3f4534b Compare April 5, 2023 14:03
@eliotwrobson
Copy link
Collaborator

I haven't had time to do a full review, but everything here looks really good (+1 for accurate type hints everywhere) and cleaner than the previous code. None of the test cases or parts of the public interface are modified, so I'm inclined to say this should be merged soon.

Copy link
Collaborator

@eliotwrobson eliotwrobson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor comments. Nothing major.

@caleb531
Copy link
Owner

caleb531 commented Apr 6, 2023

I've resolved the remaining conversation threads by just committing those small tweaks myself. @EduardoGoulart1 did you have any other changes you wanted to make to this PR before I perform a final review / merge?

cc @eliotwrobson

@EduardoGoulart1
Copy link
Contributor Author

I've resolved the remaining conversation threads by just committing those small tweaks myself. @EduardoGoulart1 did you have any other changes you wanted to make to this PR before I perform a final review / merge?

cc @eliotwrobson

No... Go ahead!

This will make subclassing DFA more flexible.
@caleb531 caleb531 force-pushed the simplify-binary-operations branch from 2f4c4e7 to da0c141 Compare April 11, 2023 19:23
@caleb531 caleb531 merged commit a1e7e3e into caleb531:develop Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants