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

Issue 137 - Fix Hyperscan duplicate editions bug #138

Conversation

mattdahl
Copy link
Contributor

Bug is described in #137. In theory, I think this could affect other tokenizers too, but I only see the problem in HyperscanTokenizer. My proposed solution is simple -- just remove duplicate candidate editions after citation tokens are merged. If there is some reason why those duplicates should be retained, I'm missing it.

@mlissner
Copy link
Member

Seems simple enough, but a number of tests are failing. Any idea what's going on there?

@mattdahl
Copy link
Contributor Author

Not sure what the problem is, they all passed fine on my fork (same commit hash): https://github.com/mattdahl/eyecite/actions/runs/3962150256/. Also not sure why the dependencies (fast_diff_match_patch, exrex) don't load on only some Python versions.

@flooie
Copy link
Contributor

flooie commented Jan 20, 2023

@mattdahl @mlissner - it certainly doesn't appear to be related to your PR. I re-ran tests for python 3.8 for a branch that passed tests yesterday (I think) to see if it would happen on it now fails. So somewhere downstream something has changed.

@albertisfu albertisfu mentioned this pull request Jan 26, 2023
@mattdahl
Copy link
Contributor Author

Can this be reviewed/merged now that #140 has fixed the CI problem? (Though note that now another CI problem will need to be fixed first in #143)

@mlissner
Copy link
Member

I synced with main and tests are re-running. If they pass, we'll be in business, I think.

@mlissner mlissner merged commit 743b3b3 into freelawproject:main Feb 24, 2023
@mlissner
Copy link
Member

Thanks for bumping this Matt! Merged!

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.

3 participants