-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
Fix court string matching with whitespace #144
base: main
Are you sure you want to change the base?
Fix court string matching with whitespace #144
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
My understanding is that in this comment #135 (comment), @flooie eliminated duplicate abbreviations from courts DB, so now it's safe to do things like this.
@flooie, wil you give this a quick review too? The only changes you need to look at are in resolve.py, the rest is updating Black.
Assuming this is good, we need to return to:
- Court name issues #129 (might be obsolete)
- Court name errors #128 (might be fixed)
- Getting full citation span #135 (don't we already support this, regardless of the code here?)
@flooie looks like this one never got your approval. Mind taking a look, please? |
Want me to rebase this? |
That'd be great, thanks @mattdahl |
94b1e2f
to
96dcc84
Compare
2d0f7cb
to
c9b4d78
Compare
Thanks. I'll take a look soon. |
As discussed in #135 (comment), there is presently a bug where court strings without whitespace are not properly matched. 3b2fe09 implements a failing test for this bug. 94b1e2f implements a simple fix.
This PR is also related to the changes proposed in #129, but I think that that proposal has been made obsolete with the removal of all the duplicate citation strings by @flooie (#135 (comment)). In any case, this PR addresses a different problem re whitespace.
Note that this PR is based off of #143 (needed to update black to make GitHub Actions happy), so that should be merged first.